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

Weird new line inside a self closing JSX tag with no attributes #3234

Closed
rpominov opened this issue Nov 10, 2017 · 7 comments
Closed

Weird new line inside a self closing JSX tag with no attributes #3234

rpominov opened this issue Nov 10, 2017 · 7 comments
Labels
lang:jsx Issues affecting JSX (not general JS issues) 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
Milestone

Comments

@rpominov
Copy link

Prettier 1.8.2
Playground link

Input:

<p>
  test test test test test test test test test test test test test test tes<br/>
  test test test test test
</p>

Output:

<p>
  test test test test test test test test test test test test test test tes<br
  />
  test test test test test
</p>;

Expected behavior:
Maybe put <br /> on the second line as a whole?

@ikatyang ikatyang added lang:jsx Issues affecting JSX (not general JS issues) type:bug Issues identifying ugly output, or a defect in the program labels Nov 10, 2017
@duailibe
Copy link
Member

duailibe commented Nov 11, 2017

This has been discussed before and it's intentionally printed this way because of a Facebook use of JSX in a translation pipeline (see #1581).

In this case, the solution is to manually include a line break before <br/> since that won't affect the runtime output and will make Prettier print correctly.

Going to close this but please comment if this does not solve your issue. Thanks for the report!

@lydell
Copy link
Member

lydell commented Nov 11, 2017

I don’t understand how this issue is the same as #1581?

@duailibe
Copy link
Member

duailibe commented Nov 11, 2017

If we put <br /> on the second line that would mean printing a new line before it (which is considered as whitespace by FB's translation pipeline) and if we print <br /> on the first line, it would exceed the print width.

@duailibe duailibe reopened this Nov 11, 2017
@duailibe
Copy link
Member

Let's keep this open until we're sure they're the same issue.

@rpominov
Copy link
Author

Yeah, I've also thought about this problem after I've wrote my original comment. Maybe the following output could be considered as a possible solution?

<p>
  test test test test test test test test test test test test test test 
  tes<br />
  test test test test test
</p>;

@lydell
Copy link
Member

lydell commented Nov 11, 2017

I think it is totally fine to exceed the print width here. While it is technically possible to break between <br and /> it feels very unnatural to do so. It’s like we’d break between the parens in someFunction(); just to stay within the print width. It’s better to treat a JSX element with neither props nor children as one unbreakable unit. Just like long variable names, they can overflow the maximum print width since they simply can’t be broken.

@rpominov
Copy link
Author

Thank you for the fix, @duailibe !

@suchipi suchipi added this to the 1.9 milestone Nov 28, 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 6, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:jsx Issues affecting JSX (not general JS issues) 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

5 participants