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

Trailing commas break old IE versions #2452

Merged
merged 1 commit into from
Nov 5, 2014
Merged

Trailing commas break old IE versions #2452

merged 1 commit into from
Nov 5, 2014

Conversation

Shahor
Copy link
Contributor

@Shahor Shahor commented Nov 3, 2014

Sadly, it doesn't look like there is an option (without side-effects) to check that with jshint 👎
(see jshint/jshint#136)

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@Shahor
Copy link
Contributor Author

Shahor commented Nov 3, 2014

Done!

@facebook-github-bot
Copy link

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

@jeffchan
Copy link
Contributor

jeffchan commented Nov 4, 2014

It'd be nice if this can be shipped. :) Our builds (which check for trailing commas) are failing

@zpao
Copy link
Member

zpao commented Nov 5, 2014

Yea we can do this. For our pre-built files we run them through es3ify, which strips trailing commas and quotes reserved words. We don't run this for the npm package though (apart from dist/ which is just the pre-built files).

We may end up going in the other direction and sync out a transform that strips trailing commas as part of the earlier build step. At FB we've actually moved in the opposite direction and don't discourage trailing commas (they do help maintain blame and it's standard procedure in our PHP code).

zpao added a commit that referenced this pull request Nov 5, 2014
Trailing commas break old IE versions
@zpao zpao merged commit 462f8ea into facebook:master Nov 5, 2014
@Shahor
Copy link
Contributor Author

Shahor commented Nov 5, 2014

Thanks for the explanation! (and the merge!)

@zpao zpao added this to the 0.12.1 milestone Nov 6, 2014
@zpao
Copy link
Member

zpao commented Nov 6, 2014

If we do a 0.12.1, we'll ship this with it.

@jeffchan
Copy link
Contributor

Looks like this didn't get shipped in 0.12.1. Can we get it into the next minor release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants