Skip to content

Commit

Permalink
Fix bundling and module resolution in the server layer (#40818)
Browse files Browse the repository at this point in the history
We currently resolve the `react-server` condition correctly inside
`externals` for the server layer, however that will cause the resolved
path to be external (as it is called "externals").

So we need a way to hook into the module resolution process to force it
to use the `react-server` condition **when it's on the server layer**.
The `resolve` option doesn't give us that ability, and the solution in
this PR is to leverage `normalModuleFactory`'s resolve hook to override
the resolve options before actually resolving it. And there we can have
the `contextInfo`.

One thing left out is bundling for the edge server, we need to add tests
and sort that out carefully.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
shuding committed Sep 27, 2022
1 parent 83c25b7 commit 406d69d
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 43 deletions.
95 changes: 56 additions & 39 deletions packages/next/build/webpack-config.ts
Expand Up @@ -1001,29 +1001,10 @@ export default async function getBaseWebpackConfig(
return `commonjs next/dist/lib/import-next-warning`
}

const resolveWithReactServerCondition =
layer === WEBPACK_LAYERS.server
? getResolve({
// If React is aliased to another channel during Next.js' local development,
// we need to provide that alias to webpack's resolver.
alias: process.env.__NEXT_REACT_CHANNEL
? {
react: `react-${process.env.__NEXT_REACT_CHANNEL}`,
'react-dom': `react-dom-${process.env.__NEXT_REACT_CHANNEL}`,
}
: false,
conditionNames: ['react-server'],
})
: null

// Special internal modules that require to be bundled for Server Components.
// Special internal modules that must be bundled for Server Components.
if (layer === WEBPACK_LAYERS.server) {
if (!isLocal && /^react(?:$|\/)/.test(request)) {
const [resolved] = await resolveWithReactServerCondition!(
context,
request
)
return resolved
if (!isLocal && /^react$/.test(request)) {
return
}
if (
request ===
Expand Down Expand Up @@ -1153,25 +1134,22 @@ export default async function getBaseWebpackConfig(
}

if (/node_modules[/\\].*\.[mc]?js$/.test(res)) {
if (
layer === WEBPACK_LAYERS.server &&
(!config.experimental?.optoutServerComponentsBundle ||
!config.experimental?.optoutServerComponentsBundle.some(
// Check if a package is opt-out of Server Components bundling.
(packageName) =>
new RegExp(`node_modules[/\\\\]${packageName}[/\\\\]`).test(res)
))
) {
try {
const [resolved] = await resolveWithReactServerCondition!(
context,
request
if (layer === WEBPACK_LAYERS.server) {
// All packages should be bundled for the server layer if they're not opted out.
if (
config.experimental.optoutServerComponentsBundle?.some(
(p: string) => {
return (
res.includes('node_modules/' + p + '/') ||
res.includes('node_modules\\' + p + '\\')
)
}
)
return `${externalType} ${resolved}`
} catch (err) {
return
// The `react-server` condition is not matched, fallback.
) {
return `${externalType} ${request}`
}

return
}

// Anything else that is standard JavaScript within `node_modules`
Expand Down Expand Up @@ -1528,6 +1506,45 @@ export default async function getBaseWebpackConfig(
},
module: {
rules: [
...(config.experimental.appDir && !isClient && !isEdgeServer
? [
{
issuerLayer: WEBPACK_LAYERS.server,
test: (req: string) => {
if (
!/\.m?js/.test(req) ||
config.experimental.optoutServerComponentsBundle?.some(
(mod) => {
return req.includes('/node_modules/' + mod + '/')
}
)
) {
return false
}
return true
},
resolve: process.env.__NEXT_REACT_CHANNEL
? {
conditionNames: ['react-server', 'node', 'require'],
alias: {
react: `react-${process.env.__NEXT_REACT_CHANNEL}`,
'react-dom': `react-dom-${process.env.__NEXT_REACT_CHANNEL}`,
},
}
: {
conditionNames: ['react-server', 'node', 'require'],
alias: {
// 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: 'react',
'react-dom': 'react-dom',
},
},
},
]
: []),

// TODO: FIXME: do NOT webpack 5 support with this
// x-ref: https://github.com/webpack/webpack/issues/11467
...(!config.experimental.fullySpecified
Expand Down
9 changes: 7 additions & 2 deletions test/e2e/app-dir/rsc-basic.test.ts
Expand Up @@ -403,9 +403,14 @@ describe('app dir - react server components', () => {
await fetchViaHTTP(next.url, '/react-server/3rd-party-package').then(
async (response) => {
const result = await resolveStreamResponse(response)
expect(result).toContain('Server: index.react-server')

// Package should be resolved based on the react-server condition,
// as well as package's dependencies.
expect(result).toContain('Server: index.react-server:react.subset')
expect(result).toContain('Client: index.default:react.full')

// Subpath exports should be resolved based on the condition too.
expect(result).toContain('Server subpath: subpath.react-server')
expect(result).toContain('Client: index.default')
expect(result).toContain('Client subpath: subpath.default')
}
)
Expand Down
@@ -1 +1,4 @@
module.exports = 'index.default'
const react = require('react')

module.exports =
'index.default:' + ('useState' in react ? 'react.full' : 'react.subset')
@@ -1 +1,4 @@
module.exports = 'index.react-server'
const react = require('react')

module.exports =
'index.react-server:' + ('useState' in react ? 'react.full' : 'react.subset')

0 comments on commit 406d69d

Please sign in to comment.