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 formatting issue with Sublime Text 3 #1789

Closed
name-k opened this issue May 28, 2017 · 12 comments
Closed

JSX formatting issue with Sublime Text 3 #1789

name-k opened this issue May 28, 2017 · 12 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Comments

@name-k
Copy link

name-k commented May 28, 2017

What I have

<p>
  one one <b>two</b>, <b>three</b> four <b>five</b> six six six six six six six six six six six six six six six.
</p>

what I get

<p>
  one one
  {' '}
  <b>two</b>
  ,
  {' '}
  <b>three</b>
  {' '}
  four
  {' '}
  <b>five</b>
  {' '}
  six six six six six six six six six six six six six six six.
</p>

Configuration

	"settings": {
	  "js_prettier": {
	    "debug": false,
	    "prettier_cli_path" : "",
	    "node_path": "",
	    "auto_format_on_save"     : true,
	    "allow_inline_formatting" : true,
	    "prettier_options": {
	      "printWidth"         : 100,
	      "tabWidth"           : 2,
	      "singleQuote"        : true,
	      "trailingComma"      : "es5",
	      "bracketSpacing"     : true,
	      "jsxBracketSameLine" : false,
	      "parser"             : "babylon",
	      "semi"               : true
	    }
	  }
	}
@vjeux
Copy link
Contributor

vjeux commented May 28, 2017

Sorry about this!

It is fixed in master, it looks like

<p>
  one one <b>two</b>, <b>three</b> four <b>five</b> six six six six six six six
  six six six six six six six six.
</p>;

We're going to do a new release next week.

@vjeux vjeux closed this as completed May 28, 2017
@name-k
Copy link
Author

name-k commented May 28, 2017

Awesome, thanks!

@name-k
Copy link
Author

name-k commented Jun 9, 2017

Please, reopen this is issue because it is not solved with latest release.

@vjeux
Copy link
Contributor

vjeux commented Jun 9, 2017

We don't remove {' '} yet (#1831 will do it, but still in the work). But if you manually undo the prettier changes, prettier won't put those ugly things back. Sorry about it!

@name-k
Copy link
Author

name-k commented Jun 9, 2017

Well, I believe it is not. I wrote this because I removed these "spaces" manually and prettier did it again. And I just updated it to the latest version for Sublime Text 3. So this bug is out there for ST3.

@vjeux
Copy link
Contributor

vjeux commented Jun 9, 2017

Can you look at what version of Sublime is using?

@name-k
Copy link
Author

name-k commented Jun 9, 2017

Sorry, it was my bad with versions, prettier was installed globally, and sublime used it instead of local version. Though, some other ugly thing happening, this

import React from 'react';
const Test = props => {
  const { foo } = props;
  return (
    <div>
      One two two two two two two two two two two two two two two two two two two {foo} three {foo} four.
    </div>
  );
};
export default Test;

becomes this

//...
    <div>
      One two two two two two two two two two two two two two two two two two two {foo} three {foo}{' '}
      four.
    </div>
//...

Depending on printWidth value {' '} is placed in a different place.
Prettier version installed is 1.4.4.

@vjeux
Copy link
Contributor

vjeux commented Jun 9, 2017

cc @karl

@karl karl reopened this Jun 9, 2017
@karl
Copy link
Collaborator

karl commented Jun 9, 2017

HI @name-k, I've done a fair amount of work on the JSX formatting, so should be able to help with this.

The output you are seeing with in your last comment is as expected. Prettier is splitting your line of text to attempt to keep it under the printWidth you have set.

There is a known issue where Prettier sometimes places the {" "} at the end of a line, making that line longer than the printWidth. Is that the problem you are reporting, or is the ugliness you are describing something else?


Interactive version of @name-k's example:

https://prettier.github.io/prettier/#%7B%22content%22%3A%22import%20React%20from%20'react'%3B%5Cnconst%20Test%20%3D%20props%20%3D%3E%20%7B%5Cn%20%20const%20%7B%20foo%20%7D%20%3D%20props%3B%5Cn%20%20return%20(%5Cn%20%20%20%20%3Cdiv%3E%5Cn%20%20%20%20%20%20One%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20%7Bfoo%7D%20three%20%7Bfoo%7D%20four.%5Cn%20%20%20%20%3C%2Fdiv%3E%5Cn%20%20)%3B%5Cn%7D%3B%5Cnexport%20default%20Test%3B%22%2C%22options%22%3A%7B%22printWidth%22%3A100%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Atrue%2C%22trailingComma%22%3A%22es5%22%2C%22bracketSpacing%22%3Atrue%2C%22jsxBracketSameLine%22%3Afalse%2C%22parser%22%3A%22babylon%22%2C%22semi%22%3Atrue%2C%22useTabs%22%3Afalse%2C%22doc%22%3Afalse%7D%7D

@name-k
Copy link
Author

name-k commented Jun 9, 2017

Hi @karl, I believe prettier should not break text nodes on spaces, inside of JSX at all. I mean, if it is several sentences, should they be broken? Normally with text nodes you expect them to stay in a single line, like common HTML text, with no workarounds to break a line like {' '}. Otherwise people who don't use prettier will have issues working with this kind of markup.

And when you make text a little longer, the behavior changes
https://prettier.github.io/prettier/#%7B%22content%22%3A%22import%20React%20from%20'react'%3B%5Cnconst%20Test%20%3D%20props%20%3D%3E%20%7B%5Cn%20%20const%20%7B%20foo%20%7D%20%3D%20props%3B%5Cn%20%20return%20(%5Cn%20%20%20%20%3Cdiv%3E%5Cn%20%20%20%20%20%20One%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20two%20%7Bfoo%7D%20three%20%7Bfoo%7D%20four.%5Cn%20%20%20%20%3C%2Fdiv%3E%5Cn%20%20)%3B%5Cn%7D%3B%5Cnexport%20default%20Test%3B%22%2C%22options%22%3A%7B%22printWidth%22%3A100%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Atrue%2C%22trailingComma%22%3A%22es5%22%2C%22bracketSpacing%22%3Atrue%2C%22jsxBracketSameLine%22%3Afalse%2C%22parser%22%3A%22babylon%22%2C%22semi%22%3Atrue%2C%22useTabs%22%3Afalse%2C%22doc%22%3Afalse%7D%7D

I assume the problem is in {variables} inside the text. I think, if it is a text node and there are variables, they should be processed as text nodes.

@karl
Copy link
Collaborator

karl commented Jun 10, 2017

We recently adding support for breaking text within JSX (#1120). This change reduced the number times we added {" "} as we could now support text and tags/expression together without forcing each node onto a separate line (#963).

The complex JSX rules mean that in order to break between text and tag/expression nodes we sometimes need to add {" "}. We haven't been able to avoid this while still printing printing JSX, if you have any ideas of how we could that please let us know!

@karl
Copy link
Collaborator

karl commented Jun 27, 2017

@name-k I'm closing this issue as I don't believe there is anything specific we need to fix. We can re-open it if there is a specific change you feel would improve Prettier.

@karl karl closed this as completed Jun 27, 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 7, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 7, 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.
Projects
None yet
Development

No branches or pull requests

3 participants