Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSX for Reason #647

Closed
wants to merge 31 commits into from
Closed

JSX for Reason #647

wants to merge 31 commits into from

Conversation

SanderSpies
Copy link
Contributor

DO NOT MERGE

What I want to get working:

let selfClosing = <Foo />;
let selfClosing = <Foo a={1} b={true} />;
let a = <Foo> <Bar c={fun a => a + 2} /> </Foo>;
let text = <Yo>"string"</Yo>;

which should be translated into something like:

let selfClosing = Foo.createElement []
let selfClosing = Foo.createElement a::1 b::true [];
let a = Foo.createElement [Bar.createElement c::(fun a => a + 2) []];
let text = Yo.createElement ["string"];

Although this PR is far from finished (I've only tested the feasibility in the parser) I'd like to get the discussion started.

Current limitations:

  • using quoted strings
  • spaced needed between > and <

What do you guys think of adding JSX support like this?

@bsansouci
Copy link
Contributor

Wow this is amazing. Thanks for doing this!

@SanderSpies
Copy link
Contributor Author

Update: this should be fairly simple to finish.

Syntax-wise I now have support for:

let selfClosing = <Foo />;
let selfClosing = <Foo a={1} b={true} />;
let a = <Foo> <Bar c={fun a => a + 2} /> </Foo>;
let a = <So> <Much> <Nesting> </Nesting> </Much> </So>;
let a = <Sibling> <One test={true}/> <Two foo={b}> </Two> </Sibling>;
let a = <Foo>"testing a string here"</Foo>;
let a = <Foo>"testing a string here" <Test /> "another string" <Bar> </Bar>{ 2 + 4 }</Foo>;

Wiring everything up is also straightforward, I just need to figure out how to perform the nesting of children within the AST.

@SanderSpies
Copy link
Contributor Author

Ready for review.

@SanderSpies SanderSpies changed the title [WIP] JSX support JSX support Jul 18, 2016
@yunxing
Copy link
Contributor

yunxing commented Jul 18, 2016

woah this is sick!

@ghost ghost added the CLA Signed label Jul 18, 2016
@jordwalke
Copy link
Member

One thing I always wanted in JSX, was the ability to put any simple expression to the right of the =.

let x = <Component count=10 point={x: 0, y:0} callBack=(fun x => x) />

@jordwalke
Copy link
Member

I'm trying to think of ways that this could conflict with custom infix operators such as:

let ( /> ) a b => a + b;

If so, we'd need to provide operator swapping support for the set of operators that conflict.

@chenglou
Copy link
Member

Big request while this is still in its early phase: prop punning please.
<Foo prop1 prop2>
This'll prevent early flux-ification where devs start pulling props out into stores for the sake of looking at less props.

@yunxing
Copy link
Contributor

yunxing commented Jul 18, 2016

How do we swap "createElement" with something else? Do we ever have the need?

@sophiebits
Copy link

sophiebits commented Jul 18, 2016

I personally see the primary advantage of JSX being the matching closing tags; the other syntax differences don't seem as valuable to me.

So I wonder if we can find something with that benefit but without special complicated syntax rules that JSX introduces (ex: whitespace parsing, entity escaping, similar spread operator but no shorthand property support). I like that your PR here already is much more minimalist than JSX.

@chenglou
Copy link
Member

chenglou commented Jul 18, 2016

@spicyj is it possible for us to eliminate the needs of jsx if we auto insert comments for "closing tags"?

normalFunc prop1::value 
  (normalFunc2 prop2::value) // "self-closing". No comment needed here really...
// normalFunc end

@ghost ghost added the CLA Signed label Jul 18, 2016
@sophiebits
Copy link

Interesting. Maybe!

@jordwalke
Copy link
Member

I pitched that idea when first building support for JSX in JS and for some reason people didn't get as excited about it as true JSX support.

@chenglou
Copy link
Member

Yeah but times have changed and people like functions more than HTML now! (jk no idea what I'm saying).

The good thing about using comments is that it provides a nice down(up?)grade path, where people who don't like matching closing tags can simply remove the comment. Also this would be editor-level rather than syntax level. But I'm sure you've heard all of these before.

@SanderSpies
Copy link
Contributor Author

@chenglou - Punning support has been added.

@jordwalke Not sure how feasible omitting { and } for attribute values is as the syntax will then conflict with things like exp GREATER exp.

@ghost ghost added the CLA Signed label Jul 19, 2016
@SanderSpies
Copy link
Contributor Author

That was actually a lot easier then I thought, replacing expr with simple_expr did the trick.

@bsansouci
Copy link
Contributor

@SanderSpies Wow you're a magician.

@ghost ghost added the CLA Signed label Jul 19, 2016
@SanderSpies
Copy link
Contributor Author

Spread operator is out, as it's too troublesome to add.

@SanderSpies
Copy link
Contributor Author

  • I've added namespace support, eg. <Namespace.Foo.Bar />.
  • Spaces are now optional between tags
  • Ensured that the operators that worked before in Reason still work.

@jordwalke
Copy link
Member

Spaces are now optional between tags

Awesome!

@SanderSpies
Copy link
Contributor Author

SanderSpies commented Jul 23, 2016

I also like to support lowercase tags, simply because writing <div> is more pleasing to my eyes then <Div>. This currently doesn't work because OCaml's module identifier is capitalized. I was wondering if we could make this work in some smart way. So far unsuccessful though, so open for suggestions.

@jordwalke
Copy link
Member

Lowercase could trivially be made into function calls, but come time to format, there's no way to know which lowercase functions to render as JSX and which ones not to. You could always parse (and therefore preserve at render time), some small attribute on those function application nodes which preserve its JSX-ness.

@SanderSpies
Copy link
Contributor Author

SanderSpies commented Aug 18, 2016

Remaining work:

  • add tests for added tokens to ensure they can be used as operators if needed
  • improve fragment support (currently only tags)

@SanderSpies
Copy link
Contributor Author

Check again please - esp. the added support for fragments.

@@ -510,6 +517,7 @@ rule token = parse
| "-" { MINUS }
| "-." { MINUSDOT }
| "<>" { LESSGREATER }
| "<><" { LESSGREATERLESS }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's no test cases for this infix operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this on the master branch at the time and it didn't work. Will try again.

@jordwalke
Copy link
Member

jordwalke commented Sep 1, 2016

Okay, I've got the printing working on top of the latest master (rebased).
There's some issues with how it's parsed though, and would require a bunch of hacky token splitting work. I'd rather not do the token splitting work if it's not necessary because the whole motivation for token splitting will go away once object types have {} syntax, so I don't want to add more use cases for token splitting when not needed. I think there's a way.
As it stands, with @SanderSpies' latest work to make jsx tokens considered simple in the parser, things like <X><Y> do not parse. (It fails on the <Y).

Here's what I believe the solution is: we need lexer rules that generate tokens for the following cases:

# One token
><Tag
# Two tokens
> <Tag

# One token
></Tag
# Two tokens
> </Tag

# One token
/></Tag
# Two tokens
/> </Tag

# One token
/><Tag
# Two tokens
/> <Tag

Where you see two tokens above one of those tokens is often one of these individual tokens:

</Tag
<Tag
/>

/> is the only token which might cause shift reduce conflicts with infix /> token, but I don't think it will. I think we can reuse the same token as /> infix without conflicts (let me know if I'm wrong). If it does cause unsolvable conflicts, then we'd have to express the infix /> as \/> but I hope it doesn't come to that.

The following should parse correctly as well:

   simple + simple
   <t /> + <t />
   <t> </t > + <t> </t >

and

   simple simple simple
   myFunc <t /> <t />

Constraints:

In order for this to work, without ruining the natural ability for people to use > and < without being confused for JSX tags, the following must be true:

  • Closing tags must be written as </t> not </t > (I think)
  • Opening tags must be written as <tag and not < tag
  • Infix < must be written as x < y not x <y
  • Infix >< must be written as x >< y not x ><y
  • Infix /></ must be written as x /></ y not x/></y.
  • Infix />< must be written as x />< y not x/></y.

and so on. Those constraints above don't require any additional work in the parser/lexer/printer - they're just things the user must keep in mind.

If this is done in the parser, then the printer that I just rebased on master should work and we can merge this PR.

@ghost ghost added the CLA Signed label Sep 1, 2016
@vjeux
Copy link

vjeux commented Sep 14, 2016

@jordwalke One thing I always wanted in JSX, was the ability to put any simple expression to the right of the =.

let x = <Component count=10 point={x: 0, y:0} callBack=(fun x => x) />

I think that this is a mistake to support this. I understand the desire to fix JSX in order to make it what you wished it was and to use Reason as a good forcing function to do so. But this is going to yet another small difference between JavaScript and Reason that's going to be in the way of people trying to read/write both languages, for no obviously good benefit than a nice to have.

I'm not part of the project so feel free to disregard my opinion, but I think that there's a higher chance of it being more successful if it's seen as a "better JavaScript" than a "friendlier Ocaml".

If you want to run with my theory, then you should strive to make the syntax a good subset of JavaScript and only deviate when there's a strong and clear reason to do so. This is the pattern we used for React Native where we tried as much as possible to make it a good subset of the web.

Right now it seems like there's a desire to make it look like JavaScript but in practice, it's a weird mix of ocaml and js but the code doesn't look anything like actual JavaScript.

Anyway, I'm happy to chat more about it and this pull request is probably not the best forum for it, sorry about it :)

@ghost ghost added the CLA Signed label Sep 14, 2016
@bobzhang
Copy link
Contributor

bobzhang commented Sep 14, 2016

but I think that there's a higher chance of it being more successful if it's seen as a "better JavaScript" than a "friendlier Ocaml".

200% agree

@jordwalke
Copy link
Member

jordwalke commented Sep 14, 2016

@vjeux It's definitely a tough balance to strike and it might take a couple of attempts to find the sweet spot. We definitely want to keep moving Reason towards JS, the good parts. But to provide a counter-argument, there can be a positive effect to having some syntactic differences and a little bit of unfamiliarity. Remember when we released React with JSX, how it immediately filtered out a ton of people who were quick to dismiss something new? I think in retrospect that was the best possible outcome, even if accidental. To the observer, it almost looked like React flopped - and we can thank JSX for that. But the truth is that there was a great community being seeded. That's not to say we should make things worse or foreign just for the sake of it. But the truth is that if people want the benefits that sound static typing brings (safety + performance (optimized compilation)), they're going to have to learn some stuff. There's just no way around it. It can't be obtained via JavaScript as we know it today - JavaScript would have to be changed into something it's not and make the programmer learn many new concepts anyways.

So maybe it's the case that having some differences in syntax is a good way to manage expectations that there will be things to learn in order to reap those benefits. It could have the same beneficial filtering effect that JSX for React had. Thoughts?

But if you have some suggestions even beyond JSX, I'd love to hear them. A few people have even requested that parenthesis be mandatory around function arguments (we're also open to that suggestion as it makes named argument syntax much better - but opinions may vary).

But for JSX, why not go the other direction? Why not work towards JSX supporting simple expressions to the right of the = sign even in JavaScript?
Also, punning is really really great and we shouldn't neglect it just because JSX doesn't support it (if I could go back in time, I would have added punning to JSX from day one - instead an attribute with no =value is transpiled as attribute=true. JS itself didn't yet have punning - now it does, so maybe it's a good time to reevaluate).

@ghost ghost added the CLA Signed label Sep 14, 2016
@jordwalke
Copy link
Member

Oh, and one other thing @vjeux. There's actually room for a couple different "layers" here. I think with what we have for Reason right now, even though it's got some inspiration from JS (the good parts), I really do believe it is a more friendly way to use OCaml since we've fixed the top ten reported syntactic pitfalls. The benefit of this mode is that it can perfectly be converted back and forth between OCaml and Reason (in theory). All function styles interop perfectly between the two worlds.

But if you're willing to give that up, there's all kinds of syntactic transforms we could do on top, even going as far as to recreate things from JS that we really believe are worse. These transforms would not be "recoverable"(you can't go back to plain Reason from them) and that would be totally okay. There's an entire spectrum we can/should explore, even if some of those solutions (that very much resemble JS) have performance drawbacks - because at the end of the day we maintain the most important thing - we have a way to write parts of our in something lower that performs better in a way that interops perfectly with all the other stuff that targets the same allocator.

@ghost ghost added the CLA Signed label Sep 14, 2016
@vjeux
Copy link

vjeux commented Sep 19, 2016

I was doing some archeology around React and you ( @jordwalke ) wrote the following in the initial diff that brought JSX to React.

Making the sugar sweeter:

  • I'd like to omit the need for braces in as many places as possible. Not sure how difficult that
    would be.

So you were indeed write saying "One thing I always wanted in JSX, was the ability to put any simple expression to the right of the =." :)

@ghost ghost added the CLA Signed label Sep 19, 2016
@SanderSpies
Copy link
Contributor Author

Closed due to #775

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.

None yet

10 participants