Parse list and list spread in JSX

[anmonteiro]

fixes #1467

[anmonteiro]

this has been broken forever 🙈

[chenglou]

What's this again?

[anmonteiro]

Printing of fragments. we wanna print the fragment's children before the fragment's siblings.

[anmonteiro]

i.e. depth first vs. breadth first

[anmonteiro]

this test:

<div> <> ...[ [1,2], [3,4] ] </> </div>

was getting formatted to

<div> <> 1 3 2 4 </> </div>

i.e. 1 3 2 4 instead of 1 2 3 4 :)

[IwanKaramazow]

Same question as above, I think I expect <div> <> [1,2] [3,4] </> </div> here. Am I wrong?

[anmonteiro]

yeah, I'm not sure. need someone else to weigh in

[IwanKaramazow]

Small question, am I wrong that I would expect <Foo> [1, 2] other </Foo> here?

[anmonteiro]

yeah I'm not sure about this one. it currently looks to me that lists are always spliced in

[chenglou]

We should not eagerly cancel out list spread + list wrapping at a syntax level, since there might be ppxes that turns the list literal into something else (e.g. react jsx ppx turns children list into array. So <Foo> ...[] </Foo> should stay as such after formatting. Though I recall the implementation for keeping it like that becomes complicated?

[anmonteiro]

that isn't the case in master however. this is the current behavior:

$ echo '<div> ...[] </div>' | refmt
<div />;

[IwanKaramazow]

Back in the days, we decided to delay the problem because of non-idempotent formatting and possible problems with the react jsx ppx (fortunately nobody uses spread lists).
See #1429 for background

[anmonteiro]

Looks like @jordwalke's comment (#1429 (comment)) is exactly the behavior that this PR maintains?

[IwanKaramazow]

Yes, I think with the current infrastructure that this is the only thing we can do.
The only thing that I can think of is the <div> ...[ [1,2], [3, 4] ]</div>. If I'm interpreting that comment right, that should format to <div> [1, 2] [3, 4] </div>

[chenglou]

<>...[]</> doesn't parse without space in-between

<>...foo</> desugaring to [@JSX] foo might be fine; no other case where anything else possibly desugars to this right? Because I'm gonna assume on the PPX side that [@JSX] foo is the fragment + children spread.

To try to invalidate the above, I can do <> ...(<> ...[] </>) </>; but that gives me ([@JSX] [@JSX] []), which is still fine. Though really, it should be ([@JSX]([@JSX] []))

Am I making sense? Essentially, it should always be the case that whichever spread you use with whichever fragment syntax, and whichever composition of those, can always be distinguished when the PPX goes through the AST. We shouldn't accidentally lose info from our syntax-level eager removal of <>...foo</> into the resulting AST.

[Drup]

I rebased and fixed quite a few things in master...Drup:jsxlist

In particular, inline lists and normal elements can now be interleaved: <Foo> i [1,2] i [3] </Foo>.
To allow that, I had to disallow array accesses as JSX children. One must writes <> (a[2]) </>. I think it's reasonable.
While I was down there, I factorized the JSX parser quite a lot. It''s shorter with the additions than it was before.

The printer is still completely incorrect, I haven't fixed it yet (and my motivation is wearing a bit thin :p).
Even in the current release, refmt is wrong with lists in JSX:

% echo "<Foo> ([1,2]) i </Foo>" | refmt
<Foo> 1 i 2 </Foo>;