Hug JSX text after multi line tags

[karl]

With the new JSX formatting that has been merged in #1120 we ensure that there is always a line break before and after any multi line tags.

Given this input

x =
  <div>
    <span>
      First
    </span>,
    <span>
      Second
    </span>
  </div>

Previous Prettier would output:

x = (
  <div>
    <span>
      First
    </span>,
    <span>
      Second
    </span>
  </div>
);

And after #1120 Prettier will output:

x = (
  <div>
    <span>
      First
    </span>
    ,
    <span>
      Second
    </span>
  </div>
);

There was some talk about tweaking Prettier to allow to the comma to follow a multi line tag (#1120 (comment)).

I've opened this issue to give us a place to discuss this and see if we can come up with a heuristic for how we could deal with this.

[karl]

From a personal point of view I really like the new behaviour as it makes it nice and clear in the examples above that the two spans are separated by ,. I worry that it would be easy to miss when it follows a multi line element.

I'd be content for the behaviour to stay as it is, but I could easily be swayed by some example code where hugging text to a multi line element would improve the formatting.

[vjeux]

The heuristic I was thinking about was to keep text without spaces attached to the tag.

For example

<div>Elements (<span>{count}</span>)</div>

the ( and ) words do not have any space before and after the tag, so we would print it as is.

[vjeux]

Okay, so there's been some new development. It turns out that we have a custom translation pipeline at Facebook that uses JSX syntax but treats whitespaces differently than JSX. It treats \n as whitespace.

So in this world,

<div>
  <fbt:param>
    First
  </fbt:param>
  ,
  <fbt:param>
    Second
  </fbt:param>
</div>
// First , Second

<div>
  <fbt:param>
    First
  </fbt:param>,
  <fbt:param>
    Second
  </fbt:param>
</div>
// First, Second

This is a really unfortunate situation, but I do believe that the fix I suggested above is good and would solve our issue inside of Facebook.

If people do want to have the , on its own line, then prettier would leave it there, otherwise it would keep it next to the element.

We are at 1/3 of the entire codebase converted and this is one of the only blocker, I'm really happy :)

[karl]

Oh wow, a version of JSX with different whitespace rules 😱 And I thought I was struggling to deal with the existing whitespace rules 😛

What is the chance of Facebook being able to change the translation pipeline to follow the standard JSX whitespace rules? Probably not very likely, but it would avoid any more of these edge cases (I can see a number of other ways that Prettier could break the translation pipeline due to the difference in whitespace behaviour). I wonder if the transform from here would help? https://facebook.github.io/react/blog/2014/02/20/react-v0.9.html#jsx-whitespace

Tweaking Prettier to keep text on the same line following a multi line element seems like a very practical short term fix.

@vjeux If this is a blocker to adoption at FB feel free to tackle it yourself, it might be a while before I have the free time to work on it!

[vjeux]

Yeah, this is very unfortunate indeed. The context is that we share the same one between XHP (the one in PHP) and JSX in order to get the same strings when they are shared.

We want to use the same semantics in the future but it's going to take some time to do the migration. In the meantime, this is making prettier output some files with different meanings which is really bad. So need to find a workaround.

As for writing it out, I'm going in vacation all of next week, so it'll have to wait. If you don't want to implement it, it's perfectly fine, I'll do it when I come back :)

[karl]

☝🏻 I have a PR that includes a fix for the issue blocking adoption at Facebook.