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.

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

Fix rest operator syntax errors #10200

Closed
wants to merge 1 commit into from
Closed

Fix rest operator syntax errors #10200

wants to merge 1 commit into from

Conversation

mlazowik
Copy link
Contributor

@mlazowik mlazowik commented Oct 1, 2016

These are caused by new syntax checking introduced by babylon.

"The single rest at the end only applies to binding let { x, ...y } = obj; and assignment ({ x, ...y } = obj)."

I'd say this really should be cherry picked into the stable branch.

Test plan

  1. install [email protected]
  2. see that things break
  3. apply patch
  4. things work
  5. make sure all instances were fixed (I used \.\.\..*,.*\n.*= in IntelliJ regex format—find all ... followed by newline followed by =)

Issue #10199

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @javache and @hedgerwang to be potential reviewers.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request. As you may know, we require contributors to sign our Contributor License Agreement, and we don't seem to have you on file and listed as active anymore. In order for us to review and merge your code, please email [email protected] with your details so we can update your status.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 1, 2016
@mlazowik
Copy link
Contributor Author

mlazowik commented Oct 2, 2016

Update: the offending commit has been temporarily reverted: babel/babylon#154

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ide
Copy link
Contributor

ide commented Oct 2, 2016

Closing since this is a bug in Babylon to my knowledge and has been fixed upstream.

@ide ide closed this Oct 2, 2016
@mlazowik
Copy link
Contributor Author

mlazowik commented Oct 2, 2016

Well, this is not a bug in Babylon. They decided to postpone this syntax check until major release as it's essentially a breaking change, but it will be there.

I guess you are closing because even if this gets merged nothing will prevent people from making this syntax error in new code?

@ide
Copy link
Contributor

ide commented Oct 2, 2016

Reopening because it's actually not clear to me how the trailing comma spec and the rest spec work together.

@ide ide reopened this Oct 2, 2016
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 2, 2016
@javache
Copy link
Member

javache commented Oct 5, 2016

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Oct 5, 2016
@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 5, 2016
@mlazowik mlazowik deleted the babel-syntax branch October 10, 2016 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants