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

[TypeScript] Add trailing comma for only arrow functions in tsx. #6190

Merged
merged 5 commits into from Jun 7, 2019

Conversation

@sosukesuzuki
Copy link
Contributor

commented Jun 7, 2019

Fix #6189 .
Prettier inserts a trailing comma to single type parameter for arrow functions in tsx, since v 1.18 (#6115 ). But, this feature inserts a trailing comma to type parameter for besides arrow functions too (e.g, function , interface ).

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@sosukesuzuki

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

Sorry, this bug is cause by my mistake.

Also, this feature does not happen on playground. I want somebody to check this on local environment.

@@ -3003,12 +3003,14 @@ function printPathNoParens(path, options, print, args) {
// Keep comma if the file extension is .tsx and
// has one type parameter that isn't extend with any types.
// Because, otherwise formatted result will be invalid as tsx.
const grandParent = path.getNode(2);
if (
parent.params &&
parent.params.length === 1 &&
options.filepath &&
options.filepath.match(/\.tsx/) &&

This comment has been minimized.

Copy link
@thorn0

thorn0 Jun 7, 2019

Contributor

While you're at it, please improve this regexp a bit: /\.tsx$/i
Also it's better to rewrite the last two lines using test: /\.tsx$/i.test(options.filepath)

This comment has been minimized.

Copy link
@sosukesuzuki

sosukesuzuki Jun 7, 2019

Author Contributor

Fix from b961cdc .

@duailibe

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Sorry, this bug is cause by my mistake.

@sosukesuzuki Don't worry about that! It's my fault that I didn't see that could happen

@duailibe duailibe merged commit 8812792 into prettier:master Jun 7, 2019

14 of 16 checks passed

Header rules No header rules processed
Details
Pages changed 1 new file uploaded
Details
Mixed content No mixed content detected
Details
Redirect rules 4 redirect rules processed
Details
deploy/netlify Deploy preview ready!
Details
prettier.prettier Build #20190607.5 succeeded
Details
prettier.prettier (Dev Lint on Linux Node v10) Dev Lint on Linux Node v10 succeeded
Details
prettier.prettier (Dev Test on Linux Node v10) Dev Test on Linux Node v10 succeeded
Details
prettier.prettier (Dev Test on Windows Node v10) Dev Test on Windows Node v10 succeeded
Details
prettier.prettier (Dev Test on macOS Node v10) Dev Test on macOS Node v10 succeeded
Details
prettier.prettier (Prod Build on Linux Node v10) Prod Build on Linux Node v10 succeeded
Details
prettier.prettier (Prod Lint on Linux Node v10) Prod Lint on Linux Node v10 succeeded
Details
prettier.prettier (Prod Pack on Linux Node v10) Prod Pack on Linux Node v10 succeeded
Details
prettier.prettier (Prod Test on macOS Node_v10) Prod Test on macOS Node_v10 succeeded
Details
prettier.prettier (Prod Test on macOS Node_v10_standalone) Prod Test on macOS Node_v10_standalone succeeded
Details
prettier.prettier (Prod Test on macOS Node_v4) Prod Test on macOS Node_v4 succeeded
Details

@lock lock bot locked as resolved and limited conversation to collaborators Sep 5, 2019

@lipis lipis added this to the 1.19 milestone Sep 11, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.