Apply react-server conditions to middleware (vercel#65424)

### What Reland vercel#57448 , add react-server condition resolving and apply server-only rules to middleware Closes NEXT-1653 Closes NEXT-3333 ### Why Middleware as the pre-routing layer that is indended to be light-weight. Since it's on edge runtime and only run on server but not on client, it doesn't need to include the client react bundles. Hence we apply `react-server` export condition, that if users import React we can only bundle server required APIs and if users use React client hooks we can error.
panteliselef · May 20, 2024 · d649e5e · d649e5e
1 parent be7dab8
commit d649e5e
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 72 deletions.
diff --git a/packages/next-swc/crates/next-core/src/next_edge/context.rs b/packages/next-swc/crates/next-core/src/next_edge/context.rs
@@ -25,6 +25,7 @@ use crate::{
     next_import_map::get_next_edge_import_map,
     next_server::context::ServerContextType,
     next_shared::resolve::{
+        get_invalid_client_only_resolve_plugin, get_invalid_styled_jsx_resolve_plugin,
         ModuleFeatureReportResolvePlugin, NextSharedRuntimeResolvePlugin,
         UnsupportedModulesResolvePlugin,
     },
@@ -99,6 +100,32 @@ pub async fn get_edge_resolve_options_context(
 
     let ty = ty.into_value();
 
+    let mut plugins = match ty {
+        ServerContextType::Pages { .. }
+        | ServerContextType::PagesApi { .. }
+        | ServerContextType::AppSSR { .. } => {
+            vec![]
+        }
+        ServerContextType::AppRSC { .. }
+        | ServerContextType::AppRoute { .. }
+        | ServerContextType::PagesData { .. }
+        | ServerContextType::Middleware { .. }
+        | ServerContextType::Instrumentation => {
+            vec![
+                Vc::upcast(get_invalid_client_only_resolve_plugin(project_path)),
+                Vc::upcast(get_invalid_styled_jsx_resolve_plugin(project_path)),
+            ]
+        }
+    };
+
+    let base_plugins = vec![
+        Vc::upcast(ModuleFeatureReportResolvePlugin::new(project_path)),
+        Vc::upcast(UnsupportedModulesResolvePlugin::new(project_path)),
+        Vc::upcast(NextSharedRuntimeResolvePlugin::new(project_path)),
+    ];
+
+    plugins.extend_from_slice(&base_plugins);
+
     // https://github.com/vercel/next.js/blob/bf52c254973d99fed9d71507a2e818af80b8ade7/packages/next/src/build/webpack-config.ts#L96-L102
     let mut custom_conditions = vec![mode.await?.condition().to_string()];
     custom_conditions.extend(
@@ -119,11 +146,7 @@ pub async fn get_edge_resolve_options_context(
         import_map: Some(next_edge_import_map),
         module: true,
         browser: true,
-        plugins: vec![
-            Vc::upcast(ModuleFeatureReportResolvePlugin::new(project_path)),
-            Vc::upcast(UnsupportedModulesResolvePlugin::new(project_path)),
-            Vc::upcast(NextSharedRuntimeResolvePlugin::new(project_path)),
-        ],
+        plugins,
         ..Default::default()
     };
 

diff --git a/packages/next-swc/crates/next-core/src/next_import_map.rs b/packages/next-swc/crates/next-core/src/next_import_map.rs
@@ -613,6 +613,7 @@ async fn insert_next_server_special_aliases(
         | ServerContextType::PagesApi { .. }
         | ServerContextType::AppRSC { .. }
         | ServerContextType::AppRoute { .. }
+        | ServerContextType::Middleware { .. }
         | ServerContextType::Instrumentation => {
             insert_exact_alias_map(
                 import_map,
@@ -637,22 +638,6 @@ async fn insert_next_server_special_aliases(
                 },
             );
         }
-        // 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.
-        ServerContextType::Middleware => {
-            insert_exact_alias_map(
-                import_map,
-                project_path,
-                indexmap! {
-                    "server-only" => "next/dist/compiled/server-only/empty".to_string(),
-                    "client-only" => "next/dist/compiled/client-only/index".to_string(),
-                    "next/dist/compiled/server-only" => "next/dist/compiled/server-only/empty".to_string(),
-                    "next/dist/compiled/client-only" => "next/dist/compiled/client-only/index".to_string(),
-                    "next/dist/compiled/client-only/error" => "next/dist/compiled/client-only/index".to_string(),
-                },
-            );
-        }
     }
 
     import_map.insert_exact_alias(

diff --git a/packages/next-swc/crates/next-core/src/next_server/context.rs b/packages/next-swc/crates/next-core/src/next_server/context.rs
@@ -105,6 +105,7 @@ impl ServerContextType {
             ServerContextType::AppRSC { .. }
                 | ServerContextType::AppRoute { .. }
                 | ServerContextType::PagesApi { .. }
+                | ServerContextType::Middleware { .. }
         )
     }
 }
@@ -200,8 +201,8 @@ pub async fn get_server_resolve_options_context(
 
     let mut plugins = match ty {
         ServerContextType::Pages { .. }
-        | ServerContextType::PagesData { .. }
-        | ServerContextType::PagesApi { .. } => {
+        | ServerContextType::PagesApi { .. }
+        | ServerContextType::PagesData { .. } => {
             vec![
                 Vc::upcast(module_feature_report_resolve_plugin),
                 Vc::upcast(unsupported_modules_resolve_plugin),
@@ -246,13 +247,13 @@ pub async fn get_server_resolve_options_context(
     // means each resolve plugin must be injected only for the context where the
     // alias resolves into the error. The alias lives in here: https://github.com/vercel/next.js/blob/0060de1c4905593ea875fa7250d4b5d5ce10897d/packages/next-swc/crates/next-core/src/next_import_map.rs#L534
     match ty {
-        ServerContextType::Pages { .. } => {
+        ServerContextType::Pages { .. } | ServerContextType::PagesApi { .. } => {
             //noop
         }
         ServerContextType::PagesData { .. }
-        | ServerContextType::PagesApi { .. }
         | ServerContextType::AppRSC { .. }
         | ServerContextType::AppRoute { .. }
+        | ServerContextType::Middleware { .. }
         | ServerContextType::Instrumentation => {
             plugins.push(Vc::upcast(invalid_client_only_resolve_plugin));
             plugins.push(Vc::upcast(invalid_styled_jsx_client_only_resolve_plugin));
@@ -261,9 +262,6 @@ pub async fn get_server_resolve_options_context(
             //[TODO] Build error in this context makes rsc-build-error.ts fail which expects runtime error code
             // looks like webpack and turbopack have different order, webpack runs rsc transform first, turbopack triggers resolve plugin first.
         }
-        ServerContextType::Middleware => {
-            //noop
-        }
     }
 
     let resolve_options_context = ResolveOptionsContext {

diff --git a/packages/next/src/build/swc/options.ts b/packages/next/src/build/swc/options.ts
@@ -6,6 +6,7 @@ import type {
   StyledComponentsConfig,
 } from '../../server/config-shared'
 import type { ResolvedBaseUrl } from '../load-jsconfig'
+import { isWebpackServerOnlyLayer } from '../utils'
 
 const nextDistPath =
   /(next[\\/]dist[\\/]shared[\\/]lib)|(next[\\/]dist[\\/]client)|(next[\\/]dist[\\/]pages)/
@@ -78,8 +79,7 @@ function getBaseSWCOptions({
   serverComponents?: boolean
   bundleLayer?: WebpackLayerName
 }) {
-  const isReactServerLayer =
-    bundleLayer === WEBPACK_LAYERS.reactServerComponents
+  const isReactServerLayer = isWebpackServerOnlyLayer(bundleLayer)
   const parserConfig = getParserOptions({ filename, jsConfig })
   const paths = jsConfig?.compilerOptions?.paths
   const enableDecorators = Boolean(

diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts
@@ -1208,7 +1208,7 @@ export default async function getBaseWebpackConfig(
           issuerLayer: {
             or: [
               ...WEBPACK_LAYERS.GROUP.serverOnly,
-              ...WEBPACK_LAYERS.GROUP.nonClientServerTarget,
+              ...WEBPACK_LAYERS.GROUP.neutralTarget,
             ],
           },
           resolve: {
@@ -1220,7 +1220,7 @@ export default async function getBaseWebpackConfig(
           issuerLayer: {
             not: [
               ...WEBPACK_LAYERS.GROUP.serverOnly,
-              ...WEBPACK_LAYERS.GROUP.nonClientServerTarget,
+              ...WEBPACK_LAYERS.GROUP.neutralTarget,
             ],
           },
           resolve: {
@@ -1252,7 +1252,7 @@ export default async function getBaseWebpackConfig(
           issuerLayer: {
             not: [
               ...WEBPACK_LAYERS.GROUP.serverOnly,
-              ...WEBPACK_LAYERS.GROUP.nonClientServerTarget,
+              ...WEBPACK_LAYERS.GROUP.neutralTarget,
             ],
           },
           options: {
@@ -1270,7 +1270,7 @@ export default async function getBaseWebpackConfig(
           ],
           loader: 'empty-loader',
           issuerLayer: {
-            or: WEBPACK_LAYERS.GROUP.nonClientServerTarget,
+            or: WEBPACK_LAYERS.GROUP.neutralTarget,
           },
         },
         ...(hasAppDir

diff --git a/packages/next/src/lib/constants.ts b/packages/next/src/lib/constants.ts
@@ -168,16 +168,16 @@ const WEBPACK_LAYERS = {
       WEBPACK_LAYERS_NAMES.appMetadataRoute,
       WEBPACK_LAYERS_NAMES.appRouteHandler,
       WEBPACK_LAYERS_NAMES.instrument,
+      WEBPACK_LAYERS_NAMES.middleware,
+    ],
+    neutralTarget: [
+      // pages api
+      WEBPACK_LAYERS_NAMES.api,
     ],
     clientOnly: [
       WEBPACK_LAYERS_NAMES.serverSideRendering,
       WEBPACK_LAYERS_NAMES.appPagesBrowser,
     ],
-    nonClientServerTarget: [
-      // middleware and pages api
-      WEBPACK_LAYERS_NAMES.middleware,
-      WEBPACK_LAYERS_NAMES.api,
-    ],
     app: [
       WEBPACK_LAYERS_NAMES.reactServerComponents,
       WEBPACK_LAYERS_NAMES.actionBrowser,

diff --git a/test/e2e/module-layer/middleware.js b/test/e2e/module-layer/middleware.js
@@ -1,7 +1,11 @@
 import 'server-only'
+import React from 'react'
 import { NextResponse } from 'next/server'
 // import './lib/mixed-lib'
 
 export function middleware(request) {
+  if (React.useState) {
+    throw new Error('React.useState should not be defined in server layer')
+  }
   return NextResponse.next()
 }
diff --git a/test/e2e/module-layer/module-layer.test.ts b/test/e2e/module-layer/module-layer.test.ts
@@ -1,7 +1,8 @@
 import { nextTestSetup } from 'e2e-utils'
+import { getRedboxSource, hasRedbox, retry } from 'next-test-utils'
 
 describe('module layer', () => {
-  const { next, isNextStart } = nextTestSetup({
+  const { next, isNextStart, isNextDev, isTurbopack } = nextTestSetup({
     files: __dirname,
   })
 
@@ -59,43 +60,47 @@ describe('module layer', () => {
     }
   }
 
-  describe('no server-only in server targets', () => {
-    const middlewareFile = 'middleware.js'
-    // const pagesApiFile = 'pages/api/hello.js'
-    let middlewareContent = ''
-    // let pagesApiContent = ''
+  if (isNextDev) {
+    describe('client packages in middleware', () => {
+      const middlewareFile = 'middleware.js'
+      let middlewareContent = ''
 
-    beforeAll(async () => {
-      await next.stop()
+      afterAll(async () => {
+        await next.patchFile(middlewareFile, middlewareContent)
+      })
 
-      middlewareContent = await next.readFile(middlewareFile)
-      // pagesApiContent = await next.readFile(pagesApiFile)
+      it('should error when import server packages in middleware', async () => {
+        const browser = await next.browser('/')
 
-      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)
 
-      // 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)
+        const existingCliOutputLength = next.cliOutput.length
+        await retry(async () => {
+          expect(await hasRedbox(browser)).toBe(true)
+          const source = await getRedboxSource(browser)
+          expect(source).toContain(
+            `'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`
+          )
+        }
+      })
     })
-    runTests()
-  })
+  }
+
   describe('with server-only in server targets', () => {
     runTests()
   })

diff --git a/test/e2e/module-layer/pages/api/hello.js b/test/e2e/module-layer/pages/api/hello.js
@@ -1,5 +1,5 @@
 import 'server-only'
 
 export default function handler(req, res) {
-  return res.send('pages/api/hello.js:')
+  return res.send('pages/api/hello.js')
 }