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 generated .pnp.js compatibility with Node 6. #6871

Merged
merged 2 commits into from
Jan 4, 2019

Conversation

rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Jan 3, 2019

Summary

Prior to these changes the generated .pnp.js file would use trailing commas for function invocations which is not allowed (and generates a parse error) under Node 6.

Test plan

I would like to add some CI testing of the generated .pnp.js files to ensure that Node 4 and Node 6 continue to work (and do not regress) until the project officially drops support for them, but I am not quite sure how to go about it.

Prior to these changes the generated `.pnp.js` file would use trailing
commas for function invocations which is not allowed (and generates a
parse error) under Node 6.

This removes the offending trailing commas...
This file does not get transpiled down for Node 4 compat like other
files, so we cannot use `"trailingComma": "all"` configuration (the
default prettier config for this repo).
@arcanis
Copy link
Member

arcanis commented Jan 4, 2019

Thanks!

Regarding the tests, the simplest would likely be to make the acceptance tests work on Node 6. It could be kept in a separate branch and be periodically updated with the new master. This way we wouldn't have to downgrade the codebase and would still benefit from an automated testing.

@arcanis arcanis merged commit c1261c9 into yarnpkg:master Jan 4, 2019
@rwjblue rwjblue deleted the fix-pnp-node-6-compat branch January 4, 2019 14:06
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.

2 participants