-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[Select] Better support Preact #18027
Conversation
Details of bundle changes.Comparing: e09870b...c3b0f51
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like a good tradeoff. I would only suggest a comment to explain for people that read the sources: why?
@glromeo This PR is ready to be tested in codesandbox. See https://codesandbox.io/s/create-react-app-qb2z7 for an example. Would you be so kind and prepare a codesandbox using preact and a Select so that we can verify it works? That would help us a lot, thanks 👍 |
Do we have an option in codesandbox to force the usage of preact instead of react by all the dependencies? If it's possible, that would be neat. |
We shouldnt have a direct dependency on react. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I could test the changes with Preact, it works. However, this won't close the Preact support chapter. For instance, the focus behavior is wrong. The native select still provide a superior UX with Preact. I would propose that allocate minimum resources to support Preact, at least, give it attention in correlation to the Preact market share: https://npm-stat.com/charts.html?package=preact,react
I think that we can accept the changes because it's hopefully, the only and last time we need to do custom work for Preact support. But I'm wondering how it's going to evolve if we start to use the low level React UI elements in the future (will Preact be able to keep up?). At the same time, as AMP moves to Preact, does it gives us an advantage to support Preact?
@glromeo Thanks |
@oliviertassinari you can see my codesandbox with preact replacing react here: https://codesandbox.io/s/preact-material-ui-select-broken-eles6?fontsize=14 I couldn't get JSX top work in that environment (didn't spend too much time), here's the interesting bit in // [...]
"alias": {
"react": "preact/compat",
"react-dom": "preact/compat"
} |
Great, Parcel to the rescue: https://codesandbox.io/s/preact-material-ui-select-broken-rmwrc. |
A possible workaround for #18006