Collapses spaces between words in JSX

[lydell]

Babel does not collapse spaces between words. Prettier 1.3.1 respects that, but latest master collapses it into 1 space.

Input:

<div>a   b</div>

Babel:

"use strict";

React.createElement(
  "div",
  null,
  "a   b"
);

Prettier 1.3.1:

<div>a   b</div>;

Prettier master (commit 6f0707a):

<div>a b</div>;

Extracted from #1658 (comment)

[karl]

It looks like Babel also preserves spaces at the beginning and end of text:

http://babeljs.io/repl/#?babili=false&evaluate=false&lineWrap=false&presets=es2015%2Creact%2Cstage-2%2Cstage-3&targets=&browsers=&builtIns=false&debug=false&code=x%20%3D%20%3Cdiv%3E%20%20%20a%20%20%20b%20%20%20%3C%2Fdiv%3E&experimental=false&loose=false&spec=false&playground=true

Input:

x = <div>   a   b   </div>

Babel:

"use strict";

x = React.createElement(
  "div",
  null,
  "   a   b   "
);

[karl]

This regression was probably caused by #1120 which splits JSX text across multiple lines.

I'm not sure at the moment how to marry up preserving whitespace between words with allowing line breaks. It will need some investigation!

[karl]

I've tested preserving spaces at the beginning and end of text and both master and 1.3.1 fail to preserve the spaces.

https://prettier.github.io/prettier/#%7B%22content%22%3A%22x%20%3D%20%3Cdiv%3E%20%20%20a%20%20%20b%20%20%20%3C%2Fdiv%3E%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3A%22none%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

[lydell]

I've been thinking about this issue. Personally, I like the behavior on master. It forces people to be explicit if they want unusual spaces (<p>{' '}text{' '}end{' '}</p> or <p>{' text end '}</p>). But I guess we want to fix this to keep our "we won't change the meaning of your code" guarantee, though (even if we already kind of break that: #1658 (comment)). What could break, though? I guess a snapshot test. You could easily just remove the extra space from the snapshot, but it might feel scary that "Prettier broke your tests". Not sure how whitespace is handled in snapshots though. And if you have white-space: pre; on an element the end user would ... simply see a little less space. But if we start straying from correctness here, where do we stop...

[karl]

I feel exactly the same about this @lydell! I'm torn between personally preferring Prettier to collapse multiple spaces as part of it's formatting and the fact that doing so wouldn't be following the JSX spec.

I've been playing around with how we might keep multiple spaces intact while still being able to reformat JSX and I think it should be possible. I don't have anything to show for it yet, but I think it should be possible in a way that follows the spec while keeping all the JSX formatting behaviour we currently have.

The generally idea would be to convert multiple spaces to {" "} elements if we need a line break before or after them.

[SimenB]

Current release of prettier changes the snapshots already.

diff --git c/src/main/webapp/modules/safeTrade/seller/payment/statusDetails/handleCancelPurchaseDetails.jsx w/src/main/webapp/modules/safeTrade/seller/payment/statusDetails/handleCancelPurchaseDetails.jsx
index 99600568fc..e192d49230 100644
--- c/src/main/webapp/modules/safeTrade/seller/payment/statusDetails/handleCancelPurchaseDetails.jsx
+++ w/src/main/webapp/modules/safeTrade/seller/payment/statusDetails/handleCancelPurchaseDetails.jsx
@@ -12,7 +12,10 @@ const HandleCancelPurchaseDetails = ({ agreement, partner, handleClickCancelPurc
         <div className="phl">
             <div className="pbl">
                 <p>
-                    Hvis du hever kjøpet, bryter du en avtale du har inngått med {partner.name}, og FINN vil tilbakebetale hele beløpet
+                    Hvis du hever kjøpet, bryter du en avtale du har inngått med
+                    {' '}
+                    {partner.name}
+                    , og FINN vil tilbakebetale hele beløpet
                     til {partner.name}.
                 </p>
                 <p>
diff --git c/src/test/js/unit/test/safeTrade/seller/payment/statusDetails/__snapshots__/handleCancelPurchaseDetails-test.jsx.snap w/src/test/js/unit/test/safeTrade/seller/payment/statusDetails/__snapshots__/handleCancelPurchaseDetails-test.jsx.snap
index 832215844d..6d1923f3ae 100644
--- c/src/test/js/unit/test/safeTrade/seller/payment/statusDetails/__snapshots__/handleCancelPurchaseDetails-test.jsx.snap
+++ w/src/test/js/unit/test/safeTrade/seller/payment/statusDetails/__snapshots__/handleCancelPurchaseDetails-test.jsx.snap
@@ -45,7 +45,8 @@ exports[`HandleCancelPurchaseDetails renders correctly 1`] = `
             className="pbl"
           >
             <p>
-              Hvis du hever kjøpet, bryter du en avtale du har inngått med 
+              Hvis du hever kjøpet, bryter du en avtale du har inngått med
+               
               Ole Brumm
               , og FINN vil tilbakebetale hele beløpet til 
               Ole Brumm

[lydell]

Good point, @SimenB! I guess there's no need to worry about that, then.

[vjeux]

Is this still an issue or has it been fixed?

[lydell]

It has not been fixed. (If we want to fix it.)

[karl]

@vjeux @lydell The PR to allow JSX lines to be recombined has tweaked how we handle multiple space characters between words/tags/expressions.

We now always collapse multiple spaces. This is officially against spec, but is means that Prettier now behaves consistently with respect to multiple spaces.

If it is important for multiple spaces to be preserved then they need to be represented as an expression (e.g. {" "}). I'm with @lydell in thinking that this a acceptable (#1661 (comment)).

Would we be happy to close this for now? We can re-open it if we find that the behaviour is causing real world problems.

[karl]

No one has complained about this for 3 months so it doesn’t seem to be an issue in practice.

I’m going to close this issue and will be happy to re-open it if any issues crop up.