-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
NextJS: Aliases react
and react-dom
like next.js
does
#23671
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sookmax
requested review from
valentinpalkovic,
kasperpeulen and
yannbf
as code owners
August 1, 2023 14:03
valentinpalkovic
requested review from
JReinhold and
cdedreuille
as code owners
August 4, 2023 15:29
ndelangen
changed the title
Next.js: add webpack aliases to resolve
Next.js: Aliases Aug 25, 2023
react
and react-dom
like Next.js does.react
and react-dom
like next.js
does
@tmeasday @valentinpalkovic @ndelangen what are the next steps for this PR? |
ndelangen
changed the title
Next.js: Aliases
NextJS: Aliases Sep 19, 2023
react
and react-dom
like next.js
doesreact
and react-dom
like next.js
does
ndelangen
approved these changes
Sep 19, 2023
@ndelangen it looks like there is an unresolved problem on this PR? |
38 tasks
ndelangen
added
the
patch:yes
Bugfix & documentation PR that need to be picked to main branch
label
Sep 20, 2023
storybook-bot
pushed a commit
that referenced
this pull request
Sep 20, 2023
NextJS: Aliases `react` and `react-dom` like `next.js` does (cherry picked from commit 2b9bf9a)
Merged
8 tasks
storybook-bot
pushed a commit
that referenced
this pull request
Sep 20, 2023
NextJS: Aliases `react` and `react-dom` like `next.js` does (cherry picked from commit 2b9bf9a)
github-actions
bot
added
the
patch:done
Patch/release PRs already cherry-picked to main/release branch
label
Sep 20, 2023
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
ci:normal
nextjs
patch:done
Patch/release PRs already cherry-picked to main/release branch
patch:yes
Bugfix & documentation PR that need to be picked to main branch
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #23661
Warning: This PR seems to break the rendering of
<InlineStory />
in docs pages. Please do not accept this PR until the issue related to the rendering of<InlineStory />
is resolved.What I did
While I was investigating #23661, I noticed
next
's webpack resolvesreact
andreact-dom
differently. Specifically, it resolves them to the pre-compiled versions that reside innext/dist/compiled/react
andnext/dist/compiled/react-dom
respectively: https://github.com/vercel/next.js/blob/ff5338ce03a3240a97a5c84f5ad5c31c0f53a6ce/packages/next/src/build/webpack-config.ts#L1370-L1379.I speculate these pre-compiled
react
andreact-dom
are from Canary channel of React, which contains some undocumented / experimental APIs that are not available in Latest channel.next
uses these experimental versions ofreact
andreact-dom
because some of their components (e.g.,next/image
) depends on the APIs provided by them.To be more specific, the root cause of #23661 is the introduction of a new dependency in
next/image
(vercel/next.js#52425) that is not available inreact-dom@latest
: https://github.com/vercel/next.js/blob/7d57784fc0585cb9e5394b982761a840a6554515/packages/next/src/client/image-component.tsx#L13. (i.e.,ReactDOM.preload()
).So I added the same aliases to
@Storybook/nextjs
so thatnext/image
doesn't break in the storybook webpack server.Unfortunately, this introduces a new bug where the
<Canvas />
(or more specifically<InlineStory />
in the Canvas) doesn't render the story component properly:React does throw an error that says
Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
when I leave the docs page and enter a story page, but other than that I'm pretty much clueless why the change I made affects the rendering of<InlineStory />
😅.So, it would be much appreciated if the core team members shed some lights in this issue so that eventually we can resolve #23661.
Thanks!
How to test
Checklist
MIGRATION.MD
Maintainers
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]
🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>