Skip to content

Commit

Permalink
Revert "Reland bunling webpack middleware changes (#66049) (#66052)"
Browse files Browse the repository at this point in the history
This reverts commit 1415609.
  • Loading branch information
ztanner committed May 31, 2024
1 parent e06fd75 commit 0a4d81a
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 151 deletions.
11 changes: 4 additions & 7 deletions packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,14 +812,11 @@ export function finalizeEntrypoint({
}
}
case COMPILER_NAMES.edgeServer: {
const layer = isApi
? WEBPACK_LAYERS.api
: isMiddlewareFilename(name) || isInstrumentation
? WEBPACK_LAYERS.middleware
: undefined

return {
layer,
layer:
isMiddlewareFilename(name) || isApi || isInstrumentation
? WEBPACK_LAYERS.middleware
: undefined,
library: { name: ['_ENTRIES', `middleware_[name]`], type: 'assign' },
runtime: EDGE_RUNTIME_WEBPACK,
asyncChunks: false,
Expand Down
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 { isWebpackBundledLayer, isWebpackServerOnlyLayer } from './utils'
import { isWebpackAppLayer, 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 = isWebpackBundledLayer(layer)
const isAppLayer = isWebpackAppLayer(layer)

// Relative requires don't need custom resolution, because they
// are relative to requests we've already resolved here.
Expand Down
13 changes: 2 additions & 11 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2256,15 +2256,6 @@ 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 @@ -2287,8 +2278,8 @@ export function isWebpackDefaultLayer(
return layer === null || layer === undefined
}

export function isWebpackBundledLayer(
export function isWebpackAppLayer(
layer: WebpackLayerName | null | undefined
): boolean {
return Boolean(layer && WEBPACK_LAYERS.GROUP.bundled.includes(layer as any))
return Boolean(layer && WEBPACK_LAYERS.GROUP.app.includes(layer as any))
}
33 changes: 14 additions & 19 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import { escapeStringRegexp } from '../shared/lib/escape-regexp'
import { WEBPACK_LAYERS, WEBPACK_RESOURCE_QUERIES } from '../lib/constants'
import type { WebpackLayerName } from '../lib/constants'
import {
isWebpackBuiltinReactLayer,
isWebpackBundledLayer,
isWebpackAppLayer,
isWebpackClientOnlyLayer,
isWebpackDefaultLayer,
isWebpackServerOnlyLayer,
Expand Down Expand Up @@ -410,7 +409,8 @@ export default async function getBaseWebpackConfig(
loggedIgnoredCompilerOptions = true
}

const shouldIncludeExternalDirs = config.experimental.externalDir
const shouldIncludeExternalDirs =
config.experimental.externalDir || !!config.transpilePackages
const codeCondition = {
test: { or: [/\.(tsx|ts|js|cjs|mjs|jsx)$/, /__barrel_optimize__/] },
...(shouldIncludeExternalDirs
Expand Down Expand Up @@ -543,7 +543,7 @@ export default async function getBaseWebpackConfig(
// This will cause some performance overhead but
// acceptable as Babel will not be recommended.
getSwcLoader({
serverComponents: true,
serverComponents: false,
bundleLayer: WEBPACK_LAYERS.middleware,
}),
babelLoader,
Expand Down Expand Up @@ -592,12 +592,13 @@ 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 = useSWCLoader
? getSwcLoader({
serverComponents: false,
bundleLayer: WEBPACK_LAYERS.api,
})
: defaultLoaders.babel
const apiRoutesLayerLoaders =
hasAppDir && useSWCLoader
? getSwcLoader({
serverComponents: false,
bundleLayer: WEBPACK_LAYERS.api,
})
: defaultLoaders.babel

const pageExtensions = config.pageExtensions

Expand Down Expand Up @@ -1303,7 +1304,7 @@ export default async function getBaseWebpackConfig(
test: /next[\\/]dist[\\/](esm[\\/])?server[\\/]future[\\/]route-modules[\\/]app-page[\\/]module/,
},
{
issuerLayer: isWebpackBundledLayer,
issuerLayer: isWebpackAppLayer,
resolve: {
alias: createNextApiEsmAliases(),
},
Expand All @@ -1325,7 +1326,7 @@ export default async function getBaseWebpackConfig(
...(hasAppDir && !isClient
? [
{
issuerLayer: isWebpackBuiltinReactLayer,
issuerLayer: isWebpackServerOnlyLayer,
test: {
// Resolve it if it is a source code file, and it has NOT been
// opted out of bundling.
Expand Down Expand Up @@ -1387,7 +1388,7 @@ export default async function getBaseWebpackConfig(
// Alias react for switching between default set and share subset.
oneOf: [
{
issuerLayer: isWebpackBuiltinReactLayer,
issuerLayer: isWebpackServerOnlyLayer,
test: {
// Resolve it if it is a source code file, and it has NOT been
// opted out of bundling.
Expand Down Expand Up @@ -1468,17 +1469,11 @@ 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
11 changes: 3 additions & 8 deletions packages/next/src/build/webpack/plugins/middleware-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ import type { Telemetry } from '../../../telemetry/storage'
import { traceGlobals } from '../../../trace/shared'
import { EVENT_BUILD_FEATURE_USAGE } from '../../../telemetry/events'
import { normalizeAppPath } from '../../../shared/lib/router/utils/app-paths'
import {
INSTRUMENTATION_HOOK_FILENAME,
WEBPACK_LAYERS,
} from '../../../lib/constants'
import { INSTRUMENTATION_HOOK_FILENAME } from '../../../lib/constants'
import type { CustomRoutes } from '../../../lib/load-custom-routes'
import { isInterceptionRouteRewrite } from '../../../lib/generate-interception-routes-rewrites'
import { getDynamicCodeEvaluationError } from './wellknown-errors-plugin/parse-dynamic-code-evaluation-error'
Expand Down Expand Up @@ -275,8 +272,7 @@ function buildWebpackError({
}

function isInMiddlewareLayer(parser: webpack.javascript.JavascriptParser) {
const layer = parser.state.module?.layer
return layer === WEBPACK_LAYERS.middleware || layer === WEBPACK_LAYERS.api
return parser.state.module?.layer === 'middleware'
}

function isNodeJsModule(moduleName: string) {
Expand Down Expand Up @@ -853,8 +849,7 @@ export async function handleWebpackExternalForEdgeRuntime({
getResolve: () => any
}) {
if (
(contextInfo.issuerLayer === WEBPACK_LAYERS.middleware ||
contextInfo.issuerLayer === WEBPACK_LAYERS.api) &&
contextInfo.issuerLayer === 'middleware' &&
isNodeJsModule(request) &&
!supportedEdgePolyfills.has(request)
) {
Expand Down
7 changes: 1 addition & 6 deletions packages/next/src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,6 @@ 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 @@ -179,7 +174,7 @@ const WEBPACK_LAYERS = {
WEBPACK_LAYERS_NAMES.serverSideRendering,
WEBPACK_LAYERS_NAMES.appPagesBrowser,
],
bundled: [
app: [
WEBPACK_LAYERS_NAMES.reactServerComponents,
WEBPACK_LAYERS_NAMES.actionBrowser,
WEBPACK_LAYERS_NAMES.appMetadataRoute,
Expand Down
13 changes: 2 additions & 11 deletions test/e2e/module-layer/middleware.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
import 'server-only'
import * as React from 'react'
import React from 'react'
import { NextResponse } from 'next/server'
// import './lib/mixed-lib'

export function middleware(request) {
// To avoid webpack ESM exports checking warning
const ReactObject = Object(React)
if (ReactObject.useState) {
if (React.useState) {
throw new Error('React.useState should not be defined in server layer')
}

if (request.nextUrl.pathname === '/react-version') {
return Response.json({
React: Object.keys(ReactObject),
})
}

return NextResponse.next()
}
60 changes: 15 additions & 45 deletions test/e2e/module-layer/module-layer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ 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 @@ -22,10 +18,8 @@ describe('module layer', () => {
'/app/route',
'/app/route-edge',
// pages/api
'/api/default',
'/api/default-edge',
'/api/server-only',
'/api/server-only-edge',
'/api/hello',
'/api/hello-edge',
'/api/mixed',
]

Expand All @@ -36,35 +30,6 @@ describe('module layer', () => {
})
}

it('should render installed react-server condition for middleware', async () => {
const json = await next.fetch('/react-version').then((res) => res.json())
expect(json.React).toContain('version') // basic react-server export
expect(json.React).not.toContain('useEffect') // no client api export
})

// This is for backward compatibility, don't change react usage in existing pages/api
it('should contain client react exports for pages api', async () => {
async function verifyReactExports(route, isEdge) {
const json = await next.fetch(route).then((res) => res.json())
// contain all react-server and default condition exports
expect(json.React).toContain('version')
expect(json.React).toContain('useEffect')

// contain react-dom-server default condition exports
expect(json.ReactDomServer).toContain('version')
expect(json.ReactDomServer).toContain('renderToString')
expect(json.ReactDomServer).toContain('renderToStaticMarkup')
expect(json.ReactDomServer).toContain(
isEdge ? 'renderToReadableStream' : 'renderToPipeableStream'
)
}

await verifyReactExports('/api/default', false)
await verifyReactExports('/api/default-edge', true)
await verifyReactExports('/api/server-only', false)
await verifyReactExports('/api/server-only-edge', true)
})

if (isNextStart) {
it('should log the build info properly', async () => {
const cliOutput = next.cliOutput
Expand All @@ -75,8 +40,7 @@ describe('module layer', () => {
)
expect(functionsManifest.functions).toContainKeys([
'/app/route-edge',
'/api/default-edge',
'/api/server-only-edge',
'/api/hello-edge',
'/app/client-edge',
'/app/server-edge',
])
Expand All @@ -88,10 +52,9 @@ describe('module layer', () => {
)
expect(middlewareManifest.middleware).toBeTruthy()
expect(pagesManifest).toContainKeys([
'/api/default-edge',
'/api/hello-edge',
'/pages-ssr',
'/api/default',
'/api/server-only',
'/api/hello',
])
})
}
Expand All @@ -118,15 +81,22 @@ describe('module layer', () => {
.replace("// import './lib/mixed-lib'", "import './lib/mixed-lib'")
)

const existingCliOutputLength = next.cliOutput.length
await retry(async () => {
expect(await hasRedbox(browser)).toBe(true)
const source = await getRedboxSource(browser)
expect(source).toContain(
isTurbopack
? `'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.`
: `You're importing a component that imports client-only. It only works in a Client Component but none of its parents are marked with "use client"`
`'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.`
)
})

if (!isTurbopack) {
const newCliOutput = next.cliOutput.slice(existingCliOutputLength)
expect(newCliOutput).toContain('./middleware.js')
expect(newCliOutput).toContain(
`'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component`
)
}
})
})
}
Expand Down
11 changes: 0 additions & 11 deletions test/e2e/module-layer/pages/api/default-edge.js

This file was deleted.

9 changes: 0 additions & 9 deletions test/e2e/module-layer/pages/api/default.js

This file was deleted.

7 changes: 7 additions & 0 deletions test/e2e/module-layer/pages/api/hello-edge.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import 'server-only'

export default function handler() {
return new Response('pages/api/hello-edge.js:')
}

export const runtime = 'edge'
5 changes: 5 additions & 0 deletions test/e2e/module-layer/pages/api/hello.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import 'server-only'

export default function handler(req, res) {
return res.send('pages/api/hello.js')
}
12 changes: 0 additions & 12 deletions test/e2e/module-layer/pages/api/server-only-edge.js

This file was deleted.

Loading

0 comments on commit 0a4d81a

Please sign in to comment.