JSX formatting does not de-indent closing tag with text body

[mjbvz]

From microsoft/vscode#40454 (comment)

TypeScript Version: 2.7.0-dev.20171214

Code

(
    <div>
        text
        </div>
)

Run format

Expected behavior:

(
    <div>
        text
    </div>
)

Actual behavior:
No change. Content remains:

(
    <div>
        text
        </div>
)

The </div> is de-indented if the body is a tag instead of text:

(
    <div>
        <span />
        </div>
)

[srolel]

@mhegazy I've been working on this, and it looks like this doesn't get formatted because the linebreak and whitespace between text and </div> are consumed by the scanner as a part of the SyntaxKind.JsxText token. I have a fix I can submit but I want to make sure this would be the right behavior (removing the whitespace). This does agree with JSX compilation, where the compiled string does not include any leading/trailing whitespace, so this would not have any effect on compiled code, as it should.

The fix is along the lines of stopping after the linebreak, which will make the closing </div> tag have that whitespace as leadingTrivia which will get formatted correctly.

[mhegazy]

sounds reasonable. I have not looked at the formatting scanner in a while, so i will need to look at the change, and look around to see if i can poke holes into the fix.

[orta]

It's getting late on a Friday, so I'm leaving myself some notes:

Today the text nodes inside the JSXText are this range:

 <div>
     |text
   |</div>

Which includes the trailing whitespace. To fix this bug I've explored (a):

<div>
  |text|
    </div>

and (b)

<div>
  |text
|   </div>

b is is what Mosho1 recommends above, and a seems promising but needs some finessing somewhere also.

Issues:

• It's changing some of the JS emit unpredictably:

  • React.createElement(Bar, null, " q "),
    -> React.createElement(Bar, null, " q"," "),

• It's bringing up new formatting issues:

  • This code isn't indenting the 'goodbye' on the 2nd line

    function foo() {
    return (
            <div>
    hello
    goodbye
            </div>
        )
    }
    

There's a good chance the first one is a fencepost error somewhere, but the second I've not figured out yet. WIP here

[orta]

After amending the parser, it looks like the next issues is that we don't handle JSX siblings affecting indentation: All of these spans are siblings to the divs in the AST:

(
    <div>
        <span/>
        <span/>
        <div id="x">
            <p>OK</p>
        </div>
        <span/>
        <span/>
    </div>
)

Which on master today formats to:

(
    <div>
    <span/>
    < span />
    <div id= "x" >
    <p>OK < /p>
    < /div>
    < span />
    <span/>
    < /div>
)

Which is kinda wild because that code isn't legit JSX.

[orta]

It wasn't legit JSX, because it wasn't declared to be a JSX file - doh.