Skip to content

Conversation

@IwanKaramazow
Copy link
Contributor

#1428

<Foo> ...bar </Foo>;

<Foo> ...((a) => 1) </Foo>;

Is parsed as:

(Foo.createElement ~children:bar ()) [@JSX]);;
(Foo.createElement ~children:(fun a  -> 1) ())[@JSX];;

@chenglou
Copy link
Member

chenglou commented Oct 1, 2017

Does this work with not just ...foo but also ...<div />? Aka <Foo> ...<div /></Foo> -> Foo.createElement children::(<div />) () [@JSX]

@IwanKaramazow
Copy link
Contributor Author

Supported:

<Foo> ...bar </Foo>;
<Foo> ...((a) => 1) </Foo>;
<Foo> ...<div /> </Foo>;
<Foo> ...[|a|] </Foo>;
<Foo> ...(1, 2) </Foo>;

Desugars to the following in the old syntax:

Foo.createElement children::bar () [@JSX];
Foo.createElement children::(fun a => 1) () [@JSX];
Foo.createElement children::<div /> () [@JSX];
Foo.createElement children::[|a|] () [@JSX];
Foo.createElement children::(1, 2) () [@JSX];

It **does not ** support:

<Foo> ...[x] </Foo>;

Should I add support for ...[] ?

@chenglou
Copy link
Member

chenglou commented Oct 2, 2017

Ehhh good one, I think we should? What do you think?

@jaredly
Copy link
Contributor

jaredly commented Oct 3, 2017

Yeah it would be weird to be inconsistent

@IwanKaramazow
Copy link
Contributor Author

Added the <Navbar> ...[myList] </Navbar> variant.
Note that this will reformat to a single child because their equivalent.

<Navbar> ...[myList] </Navbar>;
(Navbar.createElement ~children:[myList] ())[@JSX]
<Navbar> myList </Navbar>;
(Navbar.createElement ~children:[myList] ())[@JSX]

List syntax inside jsx children without spread operator <Navbar> [list1] [list2] </Navbar>, is broken atm & will be addressed in a different PR.

@chenglou
Copy link
Member

chenglou commented Oct 4, 2017

Hah, that's troublesome now. Because from the react jsx ppx perspective, <Foo> ...[bar] </Foo> is definitely not the same as <Foo> bar </Foo>. Latter's an array...

@chenglou
Copy link
Member

chenglou commented Oct 4, 2017

Non-idempotent formatting isn't great. If this is the only solution then I think we should just format <Foo> ...[bar] </Foo> to <Foo> bar </Foo>. And then hopefully people don't actually use the former with the reactjs jsx ppx

@jaredly
Copy link
Contributor

jaredly commented Oct 5, 2017

Hmmm yeah are you sure <Bar>...[x]</Bar> would refmt to <Bar>x</Bar>?

@IwanKaramazow IwanKaramazow force-pushed the jsx3spread branch 2 times, most recently from 33a3c2e to 03963bf Compare October 5, 2017 20:30
@jordwalke
Copy link
Member

This wouldn't be "non-idempotent" as @IwanKaramazow described. I think it would be exactly what you would expect. <X>...any_expression</X> means "the children should exactly be the expression any_expression. If you happen to supply <x>...[literal, list]</x>, then the ...[ ] was not necessary in the first place and would then be equivalent to <x>literal list</x>.

@chenglou chenglou merged commit b1cf8c1 into reasonml:master Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants