Skip to content

Fix: Arrow functions with trailing comma + return type are throwing an error when parsing#3305

Merged
amasad merged 1 commit intobabel:masterfrom
jviereck:T7052
Feb 5, 2016
Merged

Fix: Arrow functions with trailing comma + return type are throwing an error when parsing#3305
amasad merged 1 commit intobabel:masterfrom
jviereck:T7052

Conversation

@jviereck
Copy link
Copy Markdown
Contributor

See https://phabricator.babeljs.io/T7052.

A few things to highlight and I would love to get feedback on for review:

  • Where should the tests be placed? The error only occurs when using the flow and trailingFunctionCommas. I am therefore not sure if the tests should be placed under babylon or under babel-plugin-syntax-trailing-function-commas (or anywhere else)?
  • To make the parsing work I've added a new parameter to parseParenAndDistinguishExpression, which is toggled from the flow plugin in case trailingFunctionCommas is enabled. This is a bit unpleasant because now trailingFunctionCommas influences the flow plugin. I tried to come up with a way to avoid this new dependency but failed doing so. Is this a problem - and if, is there a better solution?

@codecov-io
Copy link
Copy Markdown

Current coverage is 85.00%

Merging #3305 into master will decrease coverage by -0.25% as of 624a811

@@            master   #3305   diff @@
======================================
  Files          215     215       
  Stmts        15759   15759       
  Branches      3378    3378       
  Methods          0       0       
======================================
- Hit          13436   13396    -40
- Partial        687     729    +42
+ Missed        1636    1634     -2

Review entire Coverage Diff as of 624a811

Powered by Codecov. Updated on successful CI builds.

@hzoo hzoo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Feb 2, 2016
@hzoo
Copy link
Copy Markdown
Member

hzoo commented Feb 5, 2016

Putting the tests in babylon is good.

cc @amasad thoughts? (we can just move this to a patch version)

@amasad
Copy link
Copy Markdown
Member

amasad commented Feb 5, 2016

LGTM

amasad added a commit that referenced this pull request Feb 5, 2016
Fix: Arrow functions with trailing comma + return type are throwing an error when parsing
@amasad amasad merged commit e24be1a into babel:master Feb 5, 2016
@danez
Copy link
Copy Markdown
Member

danez commented Mar 9, 2016

I think this is also related to https://phabricator.babeljs.io/T7187
and also when I use trailing comma with spread-parameter it also fails.
So this (...functions,): Test => {} fails at the comma.

@jviereck jviereck deleted the T7052 branch March 9, 2016 12:13
JacopKane pushed a commit to JacopKane/babel that referenced this pull request Jan 11, 2018
Fix: Arrow functions with trailing comma + return type are throwing an error when parsing
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants