ocsigen / tyxml

Build valid HTML and SVG documents
https://ocsigen.org/tyxml/
Other
166 stars 62 forks source link

Add JSX API #254

Closed Drup closed 4 years ago

Drup commented 4 years ago

Follow up of #247

cc @Khady @jorisgio @ulrikstrid

Drup commented 4 years ago

Alright, the tests finally pass.

Current issues:

Drup commented 4 years ago

I added the toggle. The syntax is [@@@tyxml.jsx bool] in ocaml, and [@tyxml.jsx bool]; in reason.

@Khady I'm highly tempted to change the antiquotes so that in <foo> "bar" other </foo>, other must be a list of elements instead of a single one. A single element would be inserted by ([other]) (which is kinda verbose), and hopefully at some point, we can make reason accept [other] (without parens). This would be much more consistent with the ppx, and more expressive.

Khady commented 4 years ago

Hmm I’m probably not the best person to ask as I’m not using jsx extensively. But I tend to believe that I prefer the current setup.

ulrikstrid commented 4 years ago

I think keeping somewhat close to the react ppx would be preferable. I would rather have to spread a list than always wrap things with lists.

Drup commented 4 years ago

My issue is that the spread operator is only available if it's the only child. You can't write <> "foo" ...bar </>.

Drup commented 4 years ago

Everything's green, let's merge the current version! I kept the antiquotations as before.

@Khady @ulrikstrid I'll let you test this for a week. If someone's motivated to write a reason-inclined documentation, I would appreciate it as well ^^'