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

[RFC] Custom element props #567

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[RFC] Custom element props #567

wants to merge 6 commits into from

Conversation

rickyvetter
Copy link
Contributor

@rickyvetter rickyvetter commented May 5, 2020

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
Copy link
Contributor Author

rickyvetter commented May 5, 2020

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.

@jfrolich
Copy link

jfrolich commented May 28, 2020

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?

@eWert-Online
Copy link

eWert-Online commented Jun 11, 2020

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.

@jfrolich
Copy link

jfrolich commented Jun 12, 2020

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! :)

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

3 participants