Skip to content

Commit

Permalink
Apply react-server transform and valication to middleware (#57448)
Browse files Browse the repository at this point in the history
Apply `react-server` resolving and server components invalid APIs
checking to middleware. We want to limit the react usage in in
middleware as so far it's not aimed for rendering purpose.

Another invalid case could be: if you're doing react SSR with
`renderToString` in middleware it should be disallowed. Imaging it could
send the rendered html code to client and you display it in browser. But
it might require hydration so it can be broken.

This PR will only let you import `react-server` export condition
packages.
  • Loading branch information
huozhi authored Oct 25, 2023
1 parent aa28fa5 commit 17bd232
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 43 deletions.
8 changes: 4 additions & 4 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,17 +461,17 @@ export default async function getBaseWebpackConfig(

const swcLoaderForMiddlewareLayer = useSWCLoader
? getSwcLoader({
serverComponents: false,
isReactServerLayer: false,
serverComponents: true,
isReactServerLayer: true,
})
: // 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({
serverComponents: false,
isReactServerLayer: false,
serverComponents: true,
isReactServerLayer: true,
}),
getBabelLoader(),
]
Expand Down
5 changes: 1 addition & 4 deletions packages/next/src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,9 @@ const WEBPACK_LAYERS = {
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,
],
nonClientServerTarget: [WEBPACK_LAYERS_NAMES.api],
app: [
WEBPACK_LAYERS_NAMES.reactServerComponents,
WEBPACK_LAYERS_NAMES.actionBrowser,
Expand Down
86 changes: 51 additions & 35 deletions test/e2e/module-layer/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { createNextDescribe } from 'e2e-utils'
import { getRedboxSource, hasRedbox } from 'next-test-utils'

createNextDescribe(
'module layer',
{
files: __dirname,
},
({ next, isNextStart }) => {
({ next, isNextStart, isNextDev }) => {
function runTests() {
it('should render routes marked with restriction marks without errors', async () => {
const routes = [
Expand Down Expand Up @@ -60,45 +61,60 @@ createNextDescribe(
}
}

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

beforeAll(async () => {
await next.stop()
// Should error for using mixed (with client-only) in server targets
if (isNextDev) {
describe('no server-only in server targets', () => {
const middlewareFile = 'middleware.js'
// const pagesApiFile = 'pages/api/hello.js'
let middlewareContent = ''
// let pagesApiContent = ''

middlewareContent = await next.readFile(middlewareFile)
// pagesApiContent = await next.readFile(pagesApiFile)
beforeAll(async () => {
await next.stop()

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

// await next.patchFile(
// pagesApiFile,
// pagesApiContent
// .replace("import 'server-only'", "// import 'server-only'")
// .replace(
// "// import '../../lib/mixed-lib'",
// "import '../../lib/mixed-lib'"
// )
// )
await next.patchFile(
middlewareFile,
middlewareContent
// .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)
// 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)
})

it('should error when import client-only in middleware', async () => {
const browser = await next.browser('/')

expect(await hasRedbox(browser, true)).toBe(true)
expect(await getRedboxSource(browser)).toContain(
`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", so they're Server Components by default.`
)
})
})
runTests()
})
describe('with server-only in server targets', () => {
runTests()
})
}
}
)

0 comments on commit 17bd232

Please sign in to comment.