Handle JSX whitespace separately from fbt whitespace #4717
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b1cf275 to
7308e8d
Compare
Collaborator
Author
|
This PR is now in a good place and it would be great to get some reviews, but I'm not sure who is the best person to look at this! @vjeux If you had time it would be great if you could run this PR over some code at Facebook that uses the |
Contributor
|
I'll try it out tomorrow. |
Contributor
|
I just ran it on a big chunk of our codebase and it barely changes anything (and what it changes is for the better). Should be good to go :) |
Collaborator
Author
|
Yay! That is great news 😁 |
vjeux
approved these changes
Jun 25, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is partial progress towards #2279
With this PR we now detect Facebook
fbttags and handle whitespace for them differently from other JSX tags.This lets us tweak how we format whitespace in JSX tags without breaking the Facebook translation pipeline. This means we can now fix up some small JSX formatting edge cases that have always bugged me 😀
The main benefit here is improved consistency across different but equivalent inputs.
For example the current version of Prettier leaves these unchanged (Playground link):
And with this PR they become:
When the text separating tags or expressions is just a single character we display it on a single line where possible.
With the current version of Prettier (Playground link):
With this PR:
And finally I feel this PR improves the output of edges cases like this:
Input:
Output in current version of Prettier (Playground link):
Output with this PR: