General JSX Improvements

[bspaulding]

Remaining

• Expand last argument of arrow function

const els = items.map(item => (
  <div className="whatever">
    <span>{children}</span>
  </div>
));

Right now turns to

const els = items.map(
  item => (
    <div className="whatever">
      <span>{children}</span>
    </div>
  )
);

• Wrong indent for arrow functions inside of {}

<Something>
  {() => (
      <SomethingElse>
        <span />
      </SomethingElse>
    )}
</Something>;

• single arrays and objects should stay on the same line and not put on the next indent

<View
  style={
    [
      {
        someVeryLongStyle1: "true",
        someVeryLongStyle2: "true"
      }
    ]
  }
/>;

should be

<View
  style={[
    {
      someVeryLongStyle1: "true",
      someVeryLongStyle2: "true"
    }
  ]}
/>;

and

<View
  style={
    {
      someVeryLongStyle1: "true",
      someVeryLongStyle2: "true"
    }
  }
/>;

should be

<View
  style={{
    someVeryLongStyle1: "true",
    someVeryLongStyle2: "true"
  }}
/>;

Fixed

• Put parenthesis for bracket-less arrow functions

const StatelessComponent = ({ children }) => (
  <div className="whatever">
    <span>{children}</span>
  </div>
);

• Put parenthesis for return

function StatelessComponent({ children }) {
  return (
    <div className="whatever">
      <span>{children}</span>
    </div>
  );
}

• Puts parenthesis for variable declaration

const comp4 = (
  <div style={styles} key="something">
    Create wrapping parens and indent <strong>all the things</strong>.
  </div>
);

• Preserve space between text nodes and jsx elements in a single line

<span>
    foo <span>bar</span>
</span>

• Properly escape quotes

<div id='"' />;

should turn into

<div id="&quot;" />;

• Use double quotes for jsx attributes even with --single-quote

<div id="a" />

• Should print names with . correctly

<route.page />;

• brackets should be on the same line as the attributes

const MyComponent = (props) => 
    <div>{
        some.very.very.very.very.very.very.very.very.very.very.long.props.text
    }</div>

should be

const MyComponent = props => (
  <div>
    {some.very.very.very.very.very.very.very.very.very.very.long.props.text}
  </div>
);

(Note: the original issue has been replaced with this list of remaining items, so comments below might not make sense)

[jlongster]

The code would preferably just be function StatelessComponent({ children}) { ... }.

The above is not exactly intentional, but just how the different rules play out. Block-less arrow expressions want to be on the same line as the params. Usually you are doing things like x => x + 1. We actually have no idea about the parentheses; they are parsed away and the AST is exactly the same whether they exist or not. So the above output is natural.

At a certain point, a formatter like this is going to influence how you write code. I think the above code should be a normal function definition. But we could potentially see about specially formatting JSX that is returned from block-less arrow functions.

[bspaulding]

Dig. And point taken wrt to the arrow function not being necessary.

I'm still a bit curious why this works though:

function StatelessComponent({ children }) {
  return (
    <div className='whatever'>
      <span>{children}</span>
    </div>
  );
}

// prettier output is the same

In this case, the parens are preserved. This is a specific parser rule wrt to block-less arrows, then?

Personally, I know I use arrows in a lot of places I know I don't need to, purely for the succinctness of the implicit return. I certainly don't want to start a code-style war here, this just seems like an unexpected result at first. 🤷‍♂️

[vjeux]

Online demo if you want to try it out

https://vjeux.github.io/prettier-browser/#%7B%22content%22%3A%22const%20StatelessComponent%20%3D%20(%7B%20children%20%7D)%20%3D%3E%20(%5Cn%20%20%3Cdiv%20className%3D'whatever'%3E%5Cn%20%20%20%20%3Cspan%3E%7Bchildren%7D%3C%2Fspan%3E%5Cn%20%20%3C%2Fdiv%3E%5Cn)%3B%5Cn%5Cnfunction%20StatelessComponent(%7B%20children%20%7D)%20%7B%5Cn%20%20return%20(%5Cn%20%20%20%20%3Cdiv%20className%3D'whatever'%3E%5Cn%20%20%20%20%20%20%3Cspan%3E%7Bchildren%7D%3C%2Fspan%3E%5Cn%20%20%20%20%3C%2Fdiv%3E%5Cn%20%20)%3B%5Cn%7D%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22useFlowParser%22%3Afalse%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%7D%7D

[gaearon]

This also seems a bit unfortunate to me:

const els = items.map(item => (
  <div className='whatever'>
    <span>{children}</span>
  </div>
));

becomes

const els = items.map(
  item => <div className="whatever">
    <span>{children}</span>
  </div>
);

I would normally expect multiline JSX start and end tags to match up instead:

const els = items.map(item =>
  <div className="whatever">
    <span>{children}</span>
  </div>
);

This is a very common pattern.

[jmorrell]

I would normally expect multiline JSX start and end tags to match up instead

Is there ever a case where JSX tags don't fit one of these cases?

• match indentation level as in your example
• are on the same line
• it's a self-closing tag, in which case the terminal /> follows the above two rules

Off-hand I can't think of any 🤔 Might be a good heuristic

[jlongster]

Let's use this issue to track all JSX changes.

[vjeux]

Need to make each attribute on its own line when they overflow

<div a="1" b="2" c="3" d="4" e="5" f="6" g="7" h="8"/>

should be

<div
  a="1"
  b="2"
  c="3"
  d="4"
  e="5"
  f="6"
  g="7"
  h="8"
/>

https://vjeux.github.io/prettier-browser/#%7B%22content%22%3A%22%3Cdiv%20a%3D%5C%221%5C%22%20b%3D%5C%222%5C%22%20c%3D%5C%223%5C%22%20d%3D%5C%224%5C%22%20e%3D%5C%225%5C%22%20f%3D%5C%226%5C%22%20g%3D%5C%227%5C%22%20h%3D%5C%228%5C%22%2F%3E%22%2C%22options%22%3A%7B%22printWidth%22%3A33%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%7D%7D

[vjeux]

single quote settings should not affect jsx attributes. I've never seen anyone write <div className='a' />

<div id="a" />

should remain

<div id="a" />

even with singleQuote option enabled

https://vjeux.github.io/prettier-browser/#%7B%22content%22%3A%22%3Cdiv%20id%3D%5C%22a%5C%22%20%2F%3E%22%2C%22options%22%3A%7B%22printWidth%22%3A34%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Atrue%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%7D%7D

[vjeux]

JSX quotes must be html escaped and not string escaped.

<div id="'" />

is currently converted to invalid JSX

<div id='\'' />

it should be

<div id="&#39;" />

instead

https://vjeux.github.io/prettier-browser/#%7B%22content%22%3A%22%3Cdiv%20id%3D%5C%22'%5C%22%20%2F%3E%5Cn%20%20%22%2C%22options%22%3A%7B%22printWidth%22%3A34%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Atrue%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%7D%7D

[vramana]

There is an issue #26 tracking JSX props not breaking even after exceeding width limit. @vjeux Isn't it better to file different issues for the others?

[vjeux]

@vramana "@jlongster Let's use this issue to track all JSX changes." I'm following orders :p

[vramana]

Okay. I'll close other in favour of this issue.

[jlongster]

@vramana typically I'd say yes, but I think a lot of these can be done in a single pass to improve JSX. I worked on it some but knew that it needed another pass.

Having a lot of issues causes stress, for me at least. I like to group things when we can. Comments and JSX are two things that generally need another pass to make it solid, and after that we can file individual issues for them. Usually when features are not quite ready like these many of the issues are actually inter-related and fixing one fixes another, etc, which is why I think it makes sense to group them.

[vramana]

Copying issue #30 here

Non-Optional Spaces Removed From JSX:

Input:

<span>
    foo <span>bar</span>
</span>

Actual output

<span>
  foo<span>bar</span>
</span>;

[gaearon]

I suggest to turn the OP post into a checklist, like how we do in React: facebook/react#7925.
This lets you keep track of individual items.
You can even link to comments in the same thread.

[rattrayalex]

I just submitted #123 which addresses one of these issues. Happy to attempt some of the others later today.

[hoolymama]

Hi there,

I notice Prettier has trouble formatting a component name with a dot.
I'm using navigator in react-native.

in routes.js

import A from "./components/A"
import B from "./components/B"
const routes = { 
	a: { page: A }, 
	b: { page: B }, 
}

The offending JSX in Nav.js is something like this:

renderScene(route, navigator) {
    return <route.page />;
 }

It doesn't like route.page and gives the following error.

/usr/local/lib/node_modules/prettier/src/printer.js:1046
    ).join([ path.call(print, "object"), path.call(print, "property") ]);
      ^
TypeError: fromString(...).join is not a function
    at genericPrintNoParens (/usr/local/lib/node_modules/prettier/src/printer.js:1046:7)
    at genericPrint (/usr/local/lib/node_modules/prettier/src/printer.js:128:28)
    at p (/usr/local/lib/node_modules/prettier/src/printer.js:77:37)
    at exports.printComments (/usr/local/lib/node_modules/prettier/src/comments.js:276:17)
    at printGenerically (/usr/local/lib/node_modules/prettier/src/printer.js:77:12)
    at FastPath.call (/usr/local/lib/node_modules/prettier/src/fast-path.js:113:16)
    at genericPrintNoParens (/usr/local/lib/node_modules/prettier/src/printer.js:1106:14)
    at genericPrint (/usr/local/lib/node_modules/prettier/src/printer.js:128:28)
    at p (/usr/local/lib/node_modules/prettier/src/printer.js:77:37)
    at exports.printComments (/usr/local/lib/node_modules/prettier/src/comments.js:276:17)
[Finished in 0.3s with exit code 1]

Apologies if this should have been a separate issue.

[vramana]

@hoolymama I think this issue is already fixed on master. Which version of prettier are you using?

[hoolymama]

@vramana Ah okay, thank you
I'm on 0.0.3
I'll update and try

[hoolymama]

@vramana - Yes I updated and its fixed. Cheers!

[suchipi]

For what it's worth... I'm not a fan of function StatelessComponent({ children }) { ... } over const StatelessComponent = ({ children }) => () because adding the return with parens increases the indentation level by one compared to the arrow function version. It's pretty trivial, though.

[bspaulding]

single quote settings should not affect jsx attributes. I've never seen anyone write <div className='a' />

I've definitely seen this, usually in any react codebase where eslint was set to single quotes.

[rattrayalex]

I've mostly fixed the stateless component jsx case, just ironing out a few edge cases.

[rattrayalex]

Hmm, the challenges in #53 are interesting to me, I'm not sure how to fix them.

this:

  <Something>
    {() => (
      <SomethingElse>
        <span />
      </SomethingElse>
    )}
  </Something>

compiles to

  <Something>
    {() => (
      <SomethingElse>
        <span />
      </SomethingElse>
     )}
  </Something>

that is, the last line is aligned to the opening (, not the opening {.

Similarly, ternaries align just as they would outside of JSX, but then the } is shoved at the end of the line.

I'm not aware of a construct in prettier that can correct this... @jlongster know of anything?

Separately, ternaries should probably be prettier, but I think that's a largely unrelated problem.

[fdecampredon]

There are few things that I have noticed with attributes : http://bit.ly/2jel07w

---

Firstly I think that there should not be an extra indentation for array in this case :

Instead of :

<View style={
  [
    ...
  ]
} />

I would expect :

<View style={[
    ...
]} />

---

Secondly when an attribute content force going to next line, I think that in general most people will want the attribute itself to be on next line. so :

Instead of :

<View style={
  [
    ...
  ]
} />

I would expect :

<View 
  style={[
    ...
  ]}
/>

---

Finally when an attribute force new line inside of a tag, I would expect all attributes to be on his own line :

Instead of :

<View onClick={onClick} style={
  [
    ...
  ]
} />

I would expect :

<View 
  onClick={onClick} 
  style={[
    ...
  ]}
/>

---

I think that most of the JSX users expect the same kinds of results, but it have to be confirmed.

[knowbody]

not sure if this was mentioned somewhere, but this JSX formatting seems weird to me:

[vramana]

@knowbody This has been mentioned in several other issues.

[suchipi]

@fdecampredon explained what I've been doing perfectly

[jlongster]

Now that #123 has landed I wonder how much of this is fixed. I'd like to go through here and check all of them, and at this point probably split off into different issues. I don't have time right now, but if anyone wants to go through this issue and make a short, succinct list of issues that would be great. It would make it easier to check them off.

[vjeux]

I just updated the original comment with a list of things that have been fixed and remaining. So much progress was made!

[suchipi]

Not gonna check any off yet because I'm not sure what's been fixed, but here's a first pass at a list for everything that has been reported:

Edit: @vjeux beat me to it 😅

• Arrow function expressions returning JSX elements lose wrapping parens and don't align start/end tags and parens
• Ternaries with JSX elements lose wrapping parens and don't align start/end tags and parens
• JSX attributes don't get moved onto their own line when they overflow
• String JSX Attributes get changed to single quotes when singleQuote is enabled
• Quote characters in JSX attributes are backslash-escaped instead of HTML escaped
• Non-optional spaces are removed from JSX
• JSX Start and end tags are not always vertically aligned (end tag is at far end of content instead)
• Curly braces around a single child in JSX element are placed next to the opening and closing parent tags instead of around the child
• Extra line breaks/indentation are created for Array JSX props
• When JSX attributes become longer than one line, they are not printed on a different line from the opening tag
• When JSX attributes become longer than one line, more than one attribute can be printed on the same line

[vjeux]

@suchipi race condition!

[jlongster]

Thanks a bunch @suchipi and @vjeux! Sorry for the duplicate work. @suchipi You can cross-check your list with the one at the top and make sure everything's there.

[rattrayalex]

@vjeux can you add this one?

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.

(the good news? I have a PR mostly-written to fix this)

[rattrayalex]

oh, @vjeux may also want to add this:

if prettier splits this...

<div>
  <div>{pretend} {"there's"} {a lot of children here}</div><span>foo</span>
</div>

into this...

<div>
  <div>
    {pretend} 
    {"there's"} 
    {a lot of children here}
  </div><span>foo</span>
</div>

...we should add a line after the </div>, moving <span>foo</span> to its own line.

I may also have this partially solved, especially thanks to a recent helpful tip from @jlongster

[vjeux]

I think at this point we should close this issue and open up new individual ones for the few remaining issues. What do you think?

[knowbody]

Agreed

[rattrayalex]

Amen, I'll do that with the above.

[bspaulding]

Use double quotes for jsx attributes even with --single-quote

Sorry, (@vjeux) this is such a silly thing, I hesitate even asking, but I'd like to understand why this is becoming a rule. It seems like it only introduces complexity to the formatter and potential confusion to the user. If I say I want single quotes, why are JSX attributes special?

It just seems like a mistake to me. 🤷‍♂️ If there's legitimate reason why JSX attributes should always use double quotes, I'd love to know. 😄

[jlongster]

Yep, feel free to close this an open all new issues (sorry, it's Friday night and not going to spend a whole lot of time on this right now :))

[vjeux]

I just opened #195, #196 and #197. @rattrayalex, please create issues about the problems you mentioned :)

[vjeux]

@bspaulding Totally fair question.

From what I've seen, the vast majority of HTML written is using double quotes. The Chrome devtools displays double quotes even though you authored them as single quotes.

JSX is a bit weird because it is HTML inside of JavaScript. At Facebook, we've been following this convention of using double quotes for JSX (and XHP) in order to look closer to typically written HTML.

All the official React docs are using single quotes for JavaScript strings and double quotes for JSX attribute strings: https://facebook.github.io/react/docs/introducing-jsx.html#jsx-represents-objects

[gaearon]

I agree double quotes in JSX is a weird rule. The most compelling reason for me to use it is because the community has already converged on it with the Airbnb rule set. So at this point there is no sense forking.

[jimmyhmiller]

This may have been captured elsewhere. But when you have a ternary and the true case is a function with an object passed to it and the false is jsx, the jsx can exceed the line length. (not saying this is good code)

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22true%20%3F%20test(%7B%5Cn%20a%3A%201%5Cn%7D)%20%3A%20%3Cdiv%20a%3D%7B123412342314%7D%20b%3D%7B123412341234%7D%20c%3D%7B123412341234%7D%20d%3D%7B123412341234%7D%20e%3D%7B123412341234%7D%20f%3D%7B123412341234%7D%20g%3D%7B123412341234%7D%20%2F%3E%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D

[vjeux]

@jimmyhmiller, do you mind creating another issue for it, this way we don't need to spam the 20 people in this conversation ;)