-
-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Docs: Remove react-dom@18
warning in docs
#21197
Conversation
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 seems like a much less hacky way of handling react version support. 👏
I had a few comments, and a concern about Vite dealing with absolute paths, but I don't have a chance to test it right now, so maybe it's fine.
I love the changes <3 |
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.
code LGTM
Closes #20729
What I did
As planned:
@storybook/react-dom-shim
with two entry points.
=>/dist/react-16
) usesreact-dom@17
's code path/dist/react-18
) usesreact-dom@18
's.@storybook/react
and@storybook/addon-docs
depend on the above, import the former and use in the place ofreact-dom
.add an alias in both packages(or perhaps a preset dependency on the shim package which adds an alias), which does something along the lines of:And similar for Vite.
How to test
Render story + docs and check for react warning (and that it works) in:
legacyRootApi = true
(all of the above in WP + vite)
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]