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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(js): quote property name as es5 compatible #5157

Merged
merged 1 commit into from Sep 28, 2018

Conversation

Projects
None yet
5 participants
@koba04
Contributor

koba04 commented Sep 28, 2018

Currently, Prettier quotes property names if the name isn't es6 compatible.
It means that Prettier might strip the quote for a property name that is es6 compatible but is not es5 compatible.
So this might break scripts on ES5 environments.

For example, the following example doesn't work on some ES5 environments.
You can try the script on Node v0.12.

Prettier 1.14.3
Playground link

--parser babylon

Input:

var obj = {
 "饜姧": 'ok'
};

console.log(obj);

Output:

var obj = {
  饜姧: "ok"
};

console.log(obj);

This PR is to fix this.

  • I鈥檝e added tests to confirm my change works.
  • (If changing the API or CLI) I鈥檝e documented the changes I鈥檝e made (in the docs/ directory)
  • I鈥檝e read the contributing guidelines.

@j-f1 j-f1 requested a review from duailibe Sep 28, 2018

@lydell

lydell approved these changes Sep 28, 2018

I think this makes sense. Haven't we said before that by default we don't want to turn es5 into es6 鈥 that's the reason we won't default trailing-comma to "all"?

I think using this kind of identifiers is not very common, so it should be a non-controversial change.

So 馃憤 from me.

@duailibe duailibe merged commit 3e05f21 into prettier:master Sep 28, 2018

10 checks passed

ci/circleci: build_prod Your tests passed on CircleCI!
Details
ci/circleci: checkout_code Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node4 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node9 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_standalone Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 80%)
Details
codecov/project 96.31% (+0%) compared to d7e96ad
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@koba04

This comment has been minimized.

Contributor

koba04 commented Sep 29, 2018

Thank you!

@koba04 koba04 deleted the koba04:es5-compatible-property-name branch Sep 29, 2018

@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment