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

[JSX] When breaking lines and removing significant whitespace, add {' '} #201

Closed
rattrayalex opened this issue Jan 14, 2017 · 3 comments
Closed
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program

Comments

@rattrayalex
Copy link
Collaborator

(c/p from #73 (comment))

When prettier breaks two previously adjacent text||jsx nodes onto separate lines, and those two nodes had been separated by whitespace, {' '} (or equivalent) must be inserted.

Eg, if prettier split this up...

<div>
  hello <span>world</span> <strong>foo</strong>
</div>

into this...

<div>
  hello
  <span>world</span> 
  <strong>foo</strong>
</div>

(which it currently wouldn't, because those lines are short – but pretend they're long 😉)
then the compiled output would change from

<div>hello <span>world</span> <strong>foo</strong></div>

to

<div>hello<span>world</span><strong>foo</strong></div>

generating "helloworldfoo" instead of "hello world foo" to the user. Oops!

That's not just a little "oops" – if someone is trying to adopt prettier in a large codebase, they're going to have to quickly skim a large number of minor changes, run their tests, and cross their fingers. This kind of thing is hard to catch, almost certainly not tested by unit tests, and results in an embarrassing visual bug being shipped to users.

I have a fix mostly-written for this.

@vjeux
Copy link
Contributor

vjeux commented Jan 15, 2017

If you can fix this and the other issue you mentioned, it'll be so good! I can't wait to use it :)

@vjeux vjeux added the type:bug Issues identifying ugly output, or a defect in the program label Jan 15, 2017
@rattrayalex
Copy link
Collaborator Author

This is now mostly-done as a part of #234 , though the PR is blocked on figuring out another aspect.

The approach I've taken to solving this problem (adding a whole new node type of 'jsx-whitespace') may not get a green light, though, in which case it'd need to be rethought.

@vjeux vjeux added the JSX label Jan 17, 2017
@vjeux
Copy link
Contributor

vjeux commented Jan 19, 2017

Fixed in 0.0.9

@vjeux vjeux closed this as completed Jan 19, 2017
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 8, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 8, 2018
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. type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

2 participants