reason-react icon indicating copy to clipboard operation
reason-react copied to clipboard

[RFC] Custom element props

Open rickyvetter opened this issue 4 years ago • 4 comments

This PR starts the process of moving <div /> from getting compiled to React.createElement("div", ...) to forcing <ReactDOM.div /> which would compile to React.createElement("div", ...). This has a bunch of side-effects but from an API perspective it makes Upper- and lower-cased PPX no longer distinguished. JSX <x> just becomes sugar for grabbing a function pair x and xProps in scope.

This is just a proof of concept and a general idea. I would love to have discussion about the following:

Is this a good/worthwhile idea?

This is a major change with big consequences.

Pros:

  • better type safety and correctness when rendering DOM nodes
  • frees up <lowercase /> JSX syntax to find functions in scope

Cons:

  • large breaking change - <div /> goes to <ReactDOM.div />
  • use of DOM primitives becomes a bit harder - must add ReactDOM to all calls or encourage open, which isn't great to use widely

The best way to generate and maintain this code

This file will be absolutely massive and incredibly prone to error if I manually create all of these. What ideas do you have for initial generation and maintenance of these?

Final API

Does ReactDOM make sense for this? Should it live somewhere else?

rickyvetter avatar May 05 '20 18:05 rickyvetter

One thought for how we could do this is to use (unfortunately for React stuff we may have to fork) tyxml to be able to generate this code.

rickyvetter avatar May 05 '20 21:05 rickyvetter

I'd love the ability for components to be just functions.

Explicitly opening ReactDOM for DOM components is fine I think. It will make using ReactDOM equivalent to React-Native and other platforms where we also need to open a module to make the primitive components available. I think that is great because it equalizes the playing field, DOM is just one platform for React.

As for the generation, how are the elements + attributes defined for React itself? Perhaps we can use the same approach?

jfrolich avatar May 28 '20 04:05 jfrolich

As for the generation, how are the elements + attributes defined for React itself? @jfrolich

I think they don't have a list defined anymore.

They got rid of the attributes list in v16 (https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html) because they pretty much had the same problems ReasonReact is having with custom attributes. (#230)

For the elements they only go by casing. Everything uppercased is a component, everything lowercased is treated as a valid DOM-Element and sent to the browser. Thats why you can even render "invalid" elements like <foo></foo> to the DOM.

So we would diverge from the way React is currently working and this is, besides the huge maintainability cost, the only thing concerning me with this RFC.

eWert-Online avatar Jun 11 '20 19:06 eWert-Online

So we would diverge from the way React is currently working and this is, besides the huge maintainability cost, the only thing concerning me with this RFC.

If we can generate it from the DOM-spec somehow, it would be low-maintenance AND typesafe! :)

jfrolich avatar Jun 12 '20 10:06 jfrolich

https://github.com/reasonml/reason-react/pull/442#issuecomment-1589179464

davesnx avatar Jun 13 '23 12:06 davesnx