Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Trailing commas after rest properties should be a syntax error #310

Closed
not-an-aardvark opened this issue Jan 5, 2017 · 2 comments
Closed

Trailing commas after rest properties should be a syntax error #310

not-an-aardvark opened this issue Jan 5, 2017 · 2 comments
Labels
accepted bug

Comments

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Jan 5, 2017 •
edited 

require('espree').parse(
    'const { foo, ...bar, } = baz;',
    { ecmaVersion: 6, ecmaFeatures: { experimentalObjectRestSpread: true } }
);

Expected behavior: SyntaxError

Actual behavior: Successful parse

ESLint has received a large number of invalid comma-dangle bug reports indirectly caused by confusion from this bug. eslint/eslint#7422, eslint/eslint#7463, eslint/eslint#7502, eslint/eslint#7614, eslint/eslint#7857, eslint/eslint#8273, eslint/eslint#8289.

That said, I'm not sure we should fix this until we do a major release. Babel actually the same issue at one point and they fixed it, but they had to revert it almost immediately afterwards due to a lot of ecosystem breakage.

Still, I thought I'd make a tracking issue for it since it's incorrect behavior.
@hzoo
Copy link
Member

hzoo commented Jan 5, 2017

👍 breaking mostly due to react-native and the fact that it compiles node_modules so users themselves didn't have to have trailing commas in their own codebase for the error to happen

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 5, 2017

(See facebook/react-native#10966 for the attempt to get the RN ecosystem to fix this problem)

@avocadowastaken avocadowastaken mentioned this issue Jan 29, 2017
Closed
not-an-aardvark added a commit that referenced this issue Mar 19, 2017
@not-an-aardvark
Fix: raise error for trailing commas after rest properties (fixes #310)
ca35a74
@not-an-aardvark not-an-aardvark mentioned this issue Mar 19, 2017
Merged
abernix added a commit to meteor/jsdoc that referenced this issue Apr 12, 2017
@abernix
Pin espree =3.4.0 to avoid throw on trailing rest comma.
713aaf4 
The comma which is causing a problem here is a trailing comma after
a rest, within a function argument's parameter destructuring:

		function a({
		  a = false,
		  ...b,
		}) {
		  // ...
		}

This was previously allowed with `experimentalObjectRestSpread` enabled
but now throws an error with eslint/js#310.

I'll be removing the comma as well as it's clearly a point of contention
but this will allow the old docs to get deployed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ljharb @hzoo @not-an-aardvark