Skip to content

Commit

Permalink
Bundle the installed react for middleware (#65811)
Browse files Browse the repository at this point in the history
### What

Let `middleware` and `instrumentation` apply `react-server` exports
condition names first. When bundle the react and react-dom, bundle the
installed version instead of the built-in version.

Renamed "app" group for webpack layers to "bundled", which indicates it
will bundle all the dependencies.

### Why

Middleware and instrument are sort of isolated from app router and pages
router, if they're using react should pick up from the installed
version. Since they're in server layer so they only need to bundle the
`react-server` conditions.

x-ref: [slack
thread](https://vercel.slack.com/archives/C046HAU4H7F/p1715790385748169)
  • Loading branch information
huozhi authored May 15, 2024
1 parent a34d909 commit 082072c
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 16 deletions.
4 changes: 2 additions & 2 deletions packages/next/src/build/handle-externals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
NODE_ESM_RESOLVE_OPTIONS,
NODE_RESOLVE_OPTIONS,
} from './webpack-config'
import { isWebpackAppLayer, isWebpackServerOnlyLayer } from './utils'
import { isWebpackBundledLayer, isWebpackServerOnlyLayer } from './utils'
import { normalizePathSep } from '../shared/lib/page-path/normalize-path-sep'
const reactPackagesRegex = /^(react|react-dom|react-server-dom-webpack)($|\/)/

Expand Down Expand Up @@ -174,7 +174,7 @@ export function makeExternalHandler({
return `commonjs next/dist/lib/import-next-warning`
}

const isAppLayer = isWebpackAppLayer(layer)
const isAppLayer = isWebpackBundledLayer(layer)

// Relative requires don't need custom resolution, because they
// are relative to requests we've already resolved here.
Expand Down
13 changes: 11 additions & 2 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2238,6 +2238,15 @@ export function getSupportedBrowsers(
return MODERN_BROWSERSLIST_TARGET
}

// Use next/dist/compiled/react packages instead of installed react
export function isWebpackBuiltinReactLayer(
layer: WebpackLayerName | null | undefined
): boolean {
return Boolean(
layer && WEBPACK_LAYERS.GROUP.builtinReact.includes(layer as any)
)
}

export function isWebpackServerOnlyLayer(
layer: WebpackLayerName | null | undefined
): boolean {
Expand All @@ -2260,8 +2269,8 @@ export function isWebpackDefaultLayer(
return layer === null || layer === undefined
}

export function isWebpackAppLayer(
export function isWebpackBundledLayer(
layer: WebpackLayerName | null | undefined
): boolean {
return Boolean(layer && WEBPACK_LAYERS.GROUP.app.includes(layer as any))
return Boolean(layer && WEBPACK_LAYERS.GROUP.bundled.includes(layer as any))
}
28 changes: 17 additions & 11 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { escapeStringRegexp } from '../shared/lib/escape-regexp'
import { WEBPACK_LAYERS, WEBPACK_RESOURCE_QUERIES } from '../lib/constants'
import type { WebpackLayerName } from '../lib/constants'
import {
isWebpackAppLayer,
isWebpackBuiltinReactLayer,
isWebpackBundledLayer,
isWebpackClientOnlyLayer,
isWebpackDefaultLayer,
isWebpackServerOnlyLayer,
Expand Down Expand Up @@ -557,13 +558,12 @@ export default async function getBaseWebpackConfig(
// Loader for API routes needs to be differently configured as it shouldn't
// have RSC transpiler enabled, so syntax checks such as invalid imports won't
// be performed.
const apiRoutesLayerLoaders =
hasAppDir && useSWCLoader
? getSwcLoader({
serverComponents: false,
bundleLayer: WEBPACK_LAYERS.api,
})
: defaultLoaders.babel
const apiRoutesLayerLoaders = useSWCLoader
? getSwcLoader({
serverComponents: false,
bundleLayer: WEBPACK_LAYERS.api,
})
: defaultLoaders.babel

const pageExtensions = config.pageExtensions

Expand Down Expand Up @@ -1292,7 +1292,7 @@ export default async function getBaseWebpackConfig(
test: /next[\\/]dist[\\/](esm[\\/])?server[\\/]future[\\/]route-modules[\\/]app-page[\\/]module/,
},
{
issuerLayer: isWebpackAppLayer,
issuerLayer: isWebpackBundledLayer,
resolve: {
alias: createNextApiEsmAliases(),
},
Expand All @@ -1314,7 +1314,7 @@ export default async function getBaseWebpackConfig(
...(hasAppDir && !isClient
? [
{
issuerLayer: isWebpackServerOnlyLayer,
issuerLayer: isWebpackBuiltinReactLayer,
test: {
// Resolve it if it is a source code file, and it has NOT been
// opted out of bundling.
Expand Down Expand Up @@ -1376,7 +1376,7 @@ export default async function getBaseWebpackConfig(
// Alias react for switching between default set and share subset.
oneOf: [
{
issuerLayer: isWebpackServerOnlyLayer,
issuerLayer: isWebpackBuiltinReactLayer,
test: {
// Resolve it if it is a source code file, and it has NOT been
// opted out of bundling.
Expand Down Expand Up @@ -1457,11 +1457,17 @@ export default async function getBaseWebpackConfig(
test: codeCondition.test,
issuerLayer: WEBPACK_LAYERS.middleware,
use: middlewareLayerLoaders,
resolve: {
conditionNames: reactServerCondition,
},
},
{
test: codeCondition.test,
issuerLayer: WEBPACK_LAYERS.instrument,
use: instrumentLayerLoaders,
resolve: {
conditionNames: reactServerCondition,
},
},
...(hasAppDir
? [
Expand Down
7 changes: 6 additions & 1 deletion packages/next/src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ export type WebpackLayerName =
const WEBPACK_LAYERS = {
...WEBPACK_LAYERS_NAMES,
GROUP: {
builtinReact: [
WEBPACK_LAYERS_NAMES.reactServerComponents,
WEBPACK_LAYERS_NAMES.actionBrowser,
WEBPACK_LAYERS_NAMES.appMetadataRoute,
],
serverOnly: [
WEBPACK_LAYERS_NAMES.reactServerComponents,
WEBPACK_LAYERS_NAMES.actionBrowser,
Expand All @@ -174,7 +179,7 @@ const WEBPACK_LAYERS = {
WEBPACK_LAYERS_NAMES.serverSideRendering,
WEBPACK_LAYERS_NAMES.appPagesBrowser,
],
app: [
bundled: [
WEBPACK_LAYERS_NAMES.reactServerComponents,
WEBPACK_LAYERS_NAMES.actionBrowser,
WEBPACK_LAYERS_NAMES.appMetadataRoute,
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/module-layer/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,10 @@ export function middleware(request) {
if (React.useState) {
throw new Error('React.useState should not be defined in server layer')
}

if (request.nextUrl.pathname === '/react-version') {
return new Response(React.version)
}

return NextResponse.next()
}
9 changes: 9 additions & 0 deletions test/e2e/module-layer/module-layer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { getRedboxSource, hasRedbox, retry } from 'next-test-utils'
describe('module layer', () => {
const { next, isNextStart, isNextDev, isTurbopack } = nextTestSetup({
files: __dirname,
dependencies: {
react: '19.0.0-rc-915b914b3a-20240515',
'react-dom': '19.0.0-rc-915b914b3a-20240515',
},
})

function runTests() {
Expand All @@ -30,6 +34,11 @@ describe('module layer', () => {
})
}

it('should render installed react version for middleware', async () => {
const text = await next.fetch('/react-version').then((res) => res.text())
expect(text).toContain('19.0.0-rc')
})

if (isNextStart) {
it('should log the build info properly', async () => {
const cliOutput = next.cliOutput
Expand Down

0 comments on commit 082072c

Please sign in to comment.