Skip to content

Commit

Permalink
Disable client-only for middleware and pages api layer (#55541)
Browse files Browse the repository at this point in the history
We need to disable the default treat `middleware` and `pages/api` as
server-only, unless users explictly import "server-only" to poison it.

This will avoid the case that when a library is mixing "client-only" API
and shared components API in one bundle, and the shared API is used in
middleware or `pages/api` that might cause error. See the test case
added.

Follow up for #55394
  • Loading branch information
huozhi authored Sep 18, 2023
1 parent e4439b1 commit 7fe01bb
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 61 deletions.
39 changes: 32 additions & 7 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -926,13 +926,13 @@ export default async function getBaseWebpackConfig(
: []

const swcLoaderForMiddlewareLayer = useSWCLoader
? getSwcLoader({ hasServerComponents: false, bundleTarget: 'server' })
? getSwcLoader({ hasServerComponents: false, bundleTarget: 'default' })
: // When using Babel, we will have to use SWC to do the optimization
// for middleware to tree shake the unused default optimized imports like "next/server".
// This will cause some performance overhead but
// acceptable as Babel will not be recommended.
[
getSwcLoader({ hasServerComponents: false, bundleTarget: 'server' }),
getSwcLoader({ hasServerComponents: false, bundleTarget: 'default' }),
getBabelLoader(),
]

Expand Down Expand Up @@ -980,7 +980,7 @@ export default async function getBaseWebpackConfig(
loader: 'next-swc-loader',
options: {
...getSwcLoader().options,
bundleTarget: 'server',
bundleTarget: 'default',
hasServerComponents: false,
},
}
Expand Down Expand Up @@ -1965,6 +1965,7 @@ export default async function getBaseWebpackConfig(
'next-flight-action-entry-loader',
'next-flight-client-module-loader',
'noop-loader',
'empty-loader',
'next-middleware-loader',
'next-edge-function-loader',
'next-edge-app-route-loader',
Expand Down Expand Up @@ -2041,7 +2042,10 @@ export default async function getBaseWebpackConfig(
// Alias server-only and client-only to proper exports based on bundling layers
{
issuerLayer: {
or: WEBPACK_LAYERS.GROUP.serverTarget,
or: [
...WEBPACK_LAYERS.GROUP.server,
...WEBPACK_LAYERS.GROUP.nonClientServerTarget,
],
},
resolve: {
// Error on client-only but allow server-only
Expand All @@ -2050,12 +2054,17 @@ export default async function getBaseWebpackConfig(
'client-only$': 'next/dist/compiled/client-only/error',
'next/dist/compiled/server-only$':
'next/dist/compiled/server-only/empty',
'next/dist/compiled/client-only$':
'next/dist/compiled/client-only/error',
},
},
},
{
issuerLayer: {
not: WEBPACK_LAYERS.GROUP.serverTarget,
not: [
...WEBPACK_LAYERS.GROUP.server,
...WEBPACK_LAYERS.GROUP.nonClientServerTarget,
],
},
resolve: {
// Error on server-only but allow client-only
Expand All @@ -2077,7 +2086,7 @@ export default async function getBaseWebpackConfig(
],
loader: 'next-invalid-import-error-loader',
issuerLayer: {
or: WEBPACK_LAYERS.GROUP.serverTarget,
or: WEBPACK_LAYERS.GROUP.server,
},
options: {
message:
Expand All @@ -2091,13 +2100,29 @@ export default async function getBaseWebpackConfig(
],
loader: 'next-invalid-import-error-loader',
issuerLayer: {
not: WEBPACK_LAYERS.GROUP.serverTarget,
not: [
...WEBPACK_LAYERS.GROUP.server,
...WEBPACK_LAYERS.GROUP.nonClientServerTarget,
],
},
options: {
message:
"'server-only' cannot be imported from a Client Component module. It should only be used from a Server Component.",
},
},
// Potential the bundle introduced into middleware and api can be poisoned by client-only
// but not being used, so we disabled the `client-only` erroring on these layers.
// `server-only` is still available.
{
test: [
/^client-only$/,
/next[\\/]dist[\\/]compiled[\\/]client-only[\\/]error/,
],
loader: 'empty-loader',
issuerLayer: {
or: WEBPACK_LAYERS.GROUP.nonClientServerTarget,
},
},
...(hasAppDir
? [
{
Expand Down
4 changes: 4 additions & 0 deletions packages/next/src/build/webpack/loaders/empty-loader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import type { webpack } from 'next/dist/compiled/webpack/webpack'

const EmptyLoader: webpack.LoaderDefinitionFunction = () => 'export default {}'
export default EmptyLoader
7 changes: 1 addition & 6 deletions packages/next/src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,7 @@ const WEBPACK_LAYERS = {
WEBPACK_LAYERS_NAMES.appMetadataRoute,
WEBPACK_LAYERS_NAMES.appRouteHandler,
],
serverTarget: [
// all GROUP.server
WEBPACK_LAYERS_NAMES.reactServerComponents,
WEBPACK_LAYERS_NAMES.actionBrowser,
WEBPACK_LAYERS_NAMES.appMetadataRoute,
WEBPACK_LAYERS_NAMES.appRouteHandler,
nonClientServerTarget: [
// plus middleware and pages api
WEBPACK_LAYERS_NAMES.middleware,
WEBPACK_LAYERS_NAMES.api,
Expand Down
134 changes: 89 additions & 45 deletions test/e2e/module-layer/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,55 +6,99 @@ createNextDescribe(
files: __dirname,
},
({ next, isNextStart }) => {
it('should render routes marked with restriction marks without errors', async () => {
const routes = [
// app client components pages
'/app/client',
'/app/client-edge',
// app sever components pages
'/app/server',
'/app/server-edge',
// app routes
'/app/route',
'/app/route-edge',
// pages/api
'/api/hello',
'/api/hello-edge',
]

for (const route of routes) {
const { status } = await next.fetch(route)
expect([route, status]).toEqual([route, 200])
}
})

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

const functionsManifest = JSON.parse(
await next.readFile('.next/server/functions-config-manifest.json')
)
expect(functionsManifest.functions).toContainKeys([
'/app/route-edge',
'/api/hello-edge',
function runTests() {
it('should render routes marked with restriction marks without errors', async () => {
const routes = [
// app client components pages
'/app/client',
'/app/client-edge',
// app sever components pages
'/app/server',
'/app/server-edge',
])
const pagesManifest = JSON.parse(
await next.readFile('.next/server/pages-manifest.json')
)
const middlewareManifest = JSON.parse(
await next.readFile('.next/server/middleware-manifest.json')
)
expect(middlewareManifest.middleware).toBeTruthy()
expect(pagesManifest).toContainKeys([
'/api/hello-edge',
'/pages-ssr',
// app routes
'/app/route',
'/app/route-edge',
// pages/api
'/api/hello',
])
'/api/hello-edge',
'/api/mixed',
]

for (const route of routes) {
const { status } = await next.fetch(route)
expect([route, status]).toEqual([route, 200])
}
})

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

const functionsManifest = JSON.parse(
await next.readFile('.next/server/functions-config-manifest.json')
)
expect(functionsManifest.functions).toContainKeys([
'/app/route-edge',
'/api/hello-edge',
'/app/client-edge',
'/app/server-edge',
])
const pagesManifest = JSON.parse(
await next.readFile('.next/server/pages-manifest.json')
)
const middlewareManifest = JSON.parse(
await next.readFile('.next/server/middleware-manifest.json')
)
expect(middlewareManifest.middleware).toBeTruthy()
expect(pagesManifest).toContainKeys([
'/api/hello-edge',
'/pages-ssr',
'/api/hello',
])
})
}
}

describe('no server-only in server targets', () => {
const middlewareFile = 'middleware.js'
// const pagesApiFile = 'pages/api/hello.js'
let middlewareContent = ''
// let pagesApiContent = ''

beforeAll(async () => {
await next.stop()

middlewareContent = await next.readFile(middlewareFile)
// pagesApiContent = await next.readFile(pagesApiFile)

await next.patchFile(
middlewareFile,
middlewareContent
.replace("import 'server-only'", "// import 'server-only'")
.replace("// import './lib/mixed-lib'", "import './lib/mixed-lib'")
)

// await next.patchFile(
// pagesApiFile,
// pagesApiContent
// .replace("import 'server-only'", "// import 'server-only'")
// .replace(
// "// import '../../lib/mixed-lib'",
// "import '../../lib/mixed-lib'"
// )
// )

await next.start()
})
afterAll(async () => {
await next.patchFile(middlewareFile, middlewareContent)
// await next.patchFile(pagesApiFile, pagesApiContent)
})
runTests()
})
describe('with server-only in server targets', () => {
runTests()
})
}
)
3 changes: 3 additions & 0 deletions test/e2e/module-layer/lib/mixed-lib/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import 'client-only'

export const client = 'client:module'
3 changes: 3 additions & 0 deletions test/e2e/module-layer/lib/mixed-lib/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { shared as sharedComponentValue } from './shared'
// export but won't be consumed in the test
export { client as clientComponentValue } from './client'
1 change: 1 addition & 0 deletions test/e2e/module-layer/lib/mixed-lib/shared.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const shared = 'shared:module'
3 changes: 2 additions & 1 deletion test/e2e/module-layer/middleware.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'server-only'
import { NextResponse } from 'next/server'
// import './lib/mixed-lib'

export function middleware() {
export function middleware(request) {
return NextResponse.next()
}
2 changes: 1 addition & 1 deletion test/e2e/module-layer/pages/api/hello-edge.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'server-only'

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

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

export default function handler(req, res) {
return res.send('api/hello.js')
return res.send('pages/api/hello.js:')
}
5 changes: 5 additions & 0 deletions test/e2e/module-layer/pages/api/mixed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import '../../lib/mixed-lib'

export default function handler(req, res) {
return res.send('pages/api/mixed.js:')
}

0 comments on commit 7fe01bb

Please sign in to comment.