Skip to content

Fix Flow generic comment positioning#5290

Merged
j-f1 merged 2 commits intoprettier:masterfrom
swac:flowgenericcomments
Oct 25, 2018
Merged

Fix Flow generic comment positioning#5290
j-f1 merged 2 commits intoprettier:masterfrom
swac:flowgenericcomments

Conversation

@swac
Copy link
Copy Markdown
Contributor

@swac swac commented Oct 24, 2018

This fixes a bug in Prettier where Flow generic comments get repositioned, breaking Flow's syntax. An example:

foo /*:: <bar> */ (baz);

Currently, Prettier will rewrite this (source) as:

foo(/*:: <bar> */ baz);

This PR attempts to fix this behavior. One issue that I ran into was dealing with the Flow parser's AST that it generates from the comment syntax. Code like this:

/*:: type Props = {
  foo?: ?string,
  bar: number,
}; */

gets turned into this (source):

type Props = {
  foo?: ?string,
  bar: number
};

I wasn't able to find an easy way to have Prettier reliably traverse the code backward to see if the annotation is inside a comment block. Since it's not in the AST at all (source), the options seem limited, particularly since the comment block could have begun many lines above the type definition. For this reason, this PR only ensures the correct behavior for non-inline type definitions like these for the Babylon parser. The Flow parser's behavior here remains the same as before.

  • 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)
  • I’ve read the contributing guidelines.

Copy link
Copy Markdown
Member

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

Can you add one more test to make sure calls without arguments work too?

Playground link

Input:

foo/*:: <bar>*/()

Output:

foo/*:: <bar> */();

Comment thread tests/flow_comments/generics.js
Comment thread src/language-js/printer-estree.js
@j-f1 j-f1 merged commit 253716d into prettier:master Oct 25, 2018
@j-f1
Copy link
Copy Markdown
Member

j-f1 commented Oct 25, 2018

Thanks for contributing!

ikatyang added a commit to ikatyang/prettier that referenced this pull request Oct 25, 2018
@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018
@lock lock Bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 23, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants