Skip to content

Commit

Permalink
Fix pages react alias (#50128)
Browse files Browse the repository at this point in the history 
Fix react and react-dom aliases are not properly set for `pages/`

* Remove alias react-dom internals to `{}` in edge runtime as we're using server rendering stub as alias
* Add `react$`, `react-dom$` missing aliases
* Only apply alias to builtin react/react-dom when it's app dir assets.
* Add `react|react-dom/package.json` resolving to require-hook
* Fix hmr issue that sometimes it will perform a reload when a hard navigation is triggered
  • Loading branch information
@huozhi
huozhi committed May 22, 2023
1 parent f4bb4aa commit 6f2e140
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 63 deletions.
108 changes: 71 additions & 37 deletions packages/next/src/build/webpack-config.ts
View file
Expand Up @@ -66,6 +66,11 @@ import { NextFontManifestPlugin } from './webpack/plugins/next-font-manifest-plu
import { getSupportedBrowsers } from './utils'
import { METADATA_RESOURCE_QUERY } from './webpack/loaders/metadata/discover'

type ExcludesFalse = <T>(x: T | false) => x is T
type ClientEntries = {
[key: string]: string | string[]
}

const EXTERNAL_PACKAGES =
require('../lib/server-external-packages.json') as string[]

Expand Down Expand Up @@ -384,7 +389,42 @@ export function getDefineEnv({
}
}

type ExcludesFalse = <T>(x: T | false) => x is T
function createReactAliases(
bundledReactChannel: string,
opts: {
reactSharedSubset: boolean
reactDomServerRenderingStub: boolean
}
) {
const alias = {
react$: `next/dist/compiled/react${bundledReactChannel}`,
'react-dom$': `next/dist/compiled/react-dom${bundledReactChannel}`,
'react/jsx-runtime$': `next/dist/compiled/react${bundledReactChannel}/jsx-runtime`,
'react/jsx-dev-runtime$': `next/dist/compiled/react${bundledReactChannel}/jsx-dev-runtime`,
'react-dom/server$': `next/dist/compiled/react-dom${bundledReactChannel}/server$`,
'react-dom/server.edge$': `next/dist/compiled/react-dom${bundledReactChannel}/server.edge`,
'react-dom/server.browser$': `next/dist/compiled/react-dom${bundledReactChannel}/server.browser`,
'react-server-dom-webpack/client$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/client`,
'react-server-dom-webpack/client.edge$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/client.edge`,
'react-server-dom-webpack/server.edge$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/server.edge`,
'react-server-dom-webpack/server.node$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/server.node`,
}

if (opts.reactSharedSubset) {
alias[
'react$'
] = `next/dist/compiled/react${bundledReactChannel}/react.shared-subset`
}
// Use server rendering stub for RSC
// x-ref: https://github.com/facebook/react/pull/25436
if (opts.reactDomServerRenderingStub) {
alias[
'react-dom$'
] = `next/dist/compiled/react-dom${bundledReactChannel}/server-rendering-stub`
}

return alias
}

const devtoolRevertWarning = execOnce(
(devtool: webpack.Configuration['devtool']) => {
Expand Down Expand Up @@ -436,10 +476,6 @@ function getOptimizedAliases(): { [pkg: string]: string } {
)
}

type ClientEntries = {
[key: string]: string | string[]
}

export function attachReactRefresh(
webpackConfig: webpack.Configuration,
targetLoader: webpack.RuleSetUseItem
Expand Down Expand Up @@ -1519,10 +1555,6 @@ export default async function getBaseWebpackConfig(
'@builder.io/partytown': '{}',
'next/dist/compiled/etag': '{}',
'next/dist/compiled/chalk': '{}',
'./cjs/react-dom-server-legacy.browser.production.min.js':
'{}',
'./cjs/react-dom-server-legacy.browser.development.js':
'{}',
},
getEdgePolyfilledModules(),
handleWebpackExternalForEdgeRuntime,
Expand Down Expand Up @@ -1822,13 +1854,10 @@ export default async function getBaseWebpackConfig(
[require.resolve('next/dynamic')]: require.resolve(
'next/dist/shared/lib/app-dynamic'
),
'react/jsx-runtime$': `next/dist/compiled/react${bundledReactChannel}/jsx-runtime`,
'react/jsx-dev-runtime$': `next/dist/compiled/react${bundledReactChannel}/jsx-dev-runtime`,
'react-dom/server.edge$': `next/dist/compiled/react-dom${bundledReactChannel}/server.edge`,
'react-server-dom-webpack/client$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/client`,
'react-server-dom-webpack/client.edge$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/client.edge`,
'react-server-dom-webpack/server.edge$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/server.edge`,
'react-server-dom-webpack/server.node$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/server.node`,
...createReactAliases(bundledReactChannel, {
reactSharedSubset: false,
reactDomServerRenderingStub: false,
}),
},
},
},
Expand Down Expand Up @@ -1856,8 +1885,10 @@ export default async function getBaseWebpackConfig(
// If missing the alias override here, the default alias will be used which aliases
// react to the direct file path, not the package name. In that case the condition
// will be ignored completely.
react: `next/dist/compiled/react${bundledReactChannel}/react.shared-subset`,
'react-dom$': `next/dist/compiled/react-dom${bundledReactChannel}/server-rendering-stub`,
...createReactAliases(bundledReactChannel, {
reactSharedSubset: true,
reactDomServerRenderingStub: true,
}),
},
},
use: {
Expand Down Expand Up @@ -1920,37 +1951,40 @@ export default async function getBaseWebpackConfig(
// It needs `conditionNames` here to require the proper asset,
// when react is acting as dependency of compiled/react-dom.
alias: {
react: `next/dist/compiled/react${bundledReactChannel}/react.shared-subset`,
// Use server rendering stub for RSC
// x-ref: https://github.com/facebook/react/pull/25436
'react-dom$': `next/dist/compiled/react-dom${bundledReactChannel}/server-rendering-stub`,
},
},
},
{
issuerLayer: WEBPACK_LAYERS.client,
test: codeCondition.test,
resolve: {
alias: {
react: `next/dist/compiled/react${bundledReactChannel}`,
'react-dom$': `next/dist/compiled/react-dom${bundledReactChannel}/server-rendering-stub`,
...createReactAliases(bundledReactChannel, {
reactSharedSubset: true,
reactDomServerRenderingStub: true,
}),
},
},
},
{
test: codeCondition.test,
issuerLayer: WEBPACK_LAYERS.client,
resolve: {
alias: {
react: `next/dist/compiled/react${bundledReactChannel}`,
'react-dom$': reactProductionProfiling
? `next/dist/compiled/react-dom${bundledReactChannel}/cjs/react-dom.profiling.min`
: `next/dist/compiled/react-dom${bundledReactChannel}`,
'react-dom/client$': `next/dist/compiled/react-dom${bundledReactChannel}/client`,
...createReactAliases(bundledReactChannel, {
reactSharedSubset: false,
reactDomServerRenderingStub: true,
}),
},
},
},
],
},
{
test: codeCondition.test,
issuerLayer: WEBPACK_LAYERS.appClient,
resolve: {
alias: {
...createReactAliases(bundledReactChannel, {
// Only alias server rendering stub in client SSR layer.
reactSharedSubset: false,
reactDomServerRenderingStub: false,
}),
},
},
},
]
: []),
{
Expand Down
24 changes: 24 additions & 0 deletions packages/next/src/server/dev/hot-reloader.ts
View file
Expand Up @@ -180,6 +180,7 @@ export default class HotReloader {
private clientError: Error | null = null
private serverError: Error | null = null
private serverPrevDocumentHash: string | null
private serverChunkNames?: Set<string>
private prevChunkNames?: Set<any>
private onDemandEntries?: ReturnType<typeof onDemandEntryHandler>
private previewProps: __ApiPreviewProps
Expand Down Expand Up @@ -1050,6 +1051,7 @@ export default class HotReloader {
(err: Error) => {
this.serverError = err
this.serverStats = null
this.serverChunkNames = undefined
}
)

Expand Down Expand Up @@ -1093,16 +1095,38 @@ export default class HotReloader {
return
}

// As document chunk will change if new app pages are joined,
// since react bundle is different it will effect the chunk hash.
// So we diff the chunk changes, if there's only new app page chunk joins,
// then we don't trigger a reload by checking pages/_document chunk change.
if (this.appDir) {
const chunkNames = new Set(compilation.namedChunks.keys())
const diffChunkNames = difference<string>(
this.serverChunkNames || new Set(),
chunkNames
)

if (
diffChunkNames.length === 0 ||
diffChunkNames.every((chunkName) => chunkName.startsWith('app/'))
) {
return
}
this.serverChunkNames = chunkNames
}

// Notify reload to reload the page, as _document.js was changed (different hash)
this.send('reloadPage')
this.serverPrevDocumentHash = documentChunk.hash || null
}
)

this.multiCompiler.hooks.done.tap('NextjsHotReloaderForServer', () => {
const serverOnlyChanges = difference<string>(
changedServerPages,
changedClientPages
)

const edgeServerOnlyChanges = difference<string>(
changedEdgeServerPages,
changedClientPages
Expand Down
18 changes: 18 additions & 0 deletions packages/next/src/server/require-hook.ts
View file
Expand Up @@ -54,6 +54,16 @@ function overrideReact() {
`next/dist/compiled/react-dom-experimental/server-rendering-stub`
),
],
[
'react/package.json',
require.resolve(`next/dist/compiled/react-experimental/package.json`),
],
[
'react-dom/package.json',
require.resolve(
`next/dist/compiled/react-dom-experimental/package.json`
),
],
[
'react-dom/client',
require.resolve(`next/dist/compiled/react-dom-experimental/client`),
Expand Down Expand Up @@ -102,6 +112,10 @@ function overrideReact() {
} else {
addHookAliases([
['react', require.resolve(`next/dist/compiled/react`)],
[
'react/package.json',
require.resolve(`next/dist/compiled/react/package.json`),
],
[
'react/jsx-runtime',
require.resolve(`next/dist/compiled/react/jsx-runtime`),
Expand All @@ -114,6 +128,10 @@ function overrideReact() {
'react-dom',
require.resolve(`next/dist/compiled/react-dom/server-rendering-stub`),
],
[
'react-dom/package.json',
require.resolve(`next/dist/compiled/react-dom/package.json`),
],
[
'react-dom/client',
require.resolve(`next/dist/compiled/react-dom/client`),
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app-dir/app-middleware/pages/api/dump-headers-edge.js
View file
@@ -1,4 +1,4 @@
export const runtime = 'experimental-edge'
export const runtime = 'edge'

export default (req) => {
return Response.json(Object.fromEntries(req.headers.entries()), {
Expand Down
12 changes: 11 additions & 1 deletion test/e2e/app-dir/rsc-basic/app/app-react/page.js
View file
@@ -1,5 +1,15 @@
import React from 'react'
import ReactDOM from 'react-dom'
import ReactDOMServer from 'react-dom/server.browser'

export default function Page() {
return <div>{'version=' + React.version}</div>
return (
<div>
<p id="react">{'React.version=' + React.version}</p>
<p id="react-dom">{'ReactDOM.version=' + ReactDOM.version}</p>
<p id="react-dom-server">
{'ReactDOMServer.version=' + ReactDOMServer.version}
</p>
</div>
)
}
17 changes: 17 additions & 0 deletions test/e2e/app-dir/rsc-basic/pages/edge-pages-react.js
View file
@@ -0,0 +1,17 @@
import React from 'react'
import ReactDOM from 'react-dom'
import ReactDOMServer from 'react-dom/server'

export default function Page() {
return (
<div>
<p id="react">{'React.version=' + React.version}</p>
<p id="react-dom">{'ReactDOM.version=' + ReactDOM.version}</p>
<p id="react-dom-server">
{'ReactDOMServer.version=' + ReactDOMServer.version}
</p>
</div>
)
}

export const runtime = 'experimental-edge'
12 changes: 11 additions & 1 deletion test/e2e/app-dir/rsc-basic/pages/pages-react.js
View file
@@ -1,5 +1,15 @@
import React from 'react'
import ReactDOM from 'react-dom'
import ReactDOMServer from 'react-dom/server'

export default function Page() {
return <div>{'version=' + React.version}</div>
return (
<div>
<p id="react">{'React.version=' + React.version}</p>
<p id="react-dom">{'ReactDOM.version=' + ReactDOM.version}</p>
<p id="react-dom-server">
{'ReactDOMServer.version=' + ReactDOMServer.version}
</p>
</div>
)
}

0 comments on commit 6f2e140

Please sign in to comment.