Fix missing error when using Actions on the client layer without enab…

…ling the feature flag (#50257) If using Server Actions on the client layer without enabling the `serverActions` feature, the build should error. Add a test case to ensure #50199 works.
vercel · May 25, 2023 · 25ce787 · 25ce787
1 parent 8725b6c
commit 25ce787
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 17 deletions.
diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts
@@ -390,14 +390,15 @@ export function getDefineEnv({
   }
 }
 
-function createReactAliases(
+function createRSCAliases(
   bundledReactChannel: string,
   opts: {
     reactSharedSubset: boolean
     reactDomServerRenderingStub: boolean
+    reactServerCondition?: boolean
   }
 ) {
-  const alias = {
+  const alias: Record<string, string> = {
     react$: `next/dist/compiled/react${bundledReactChannel}`,
     'react-dom$': `next/dist/compiled/react-dom${bundledReactChannel}`,
     'react/jsx-runtime$': `next/dist/compiled/react${bundledReactChannel}/jsx-runtime`,
@@ -425,6 +426,19 @@ function createReactAliases(
     ] = `next/dist/compiled/react-dom${bundledReactChannel}/server-rendering-stub`
   }
 
+  // Alias `server-only` and `client-only` modules to their server/client only, vendored versions.
+  // These aliases are necessary if the user doesn't have those two packages installed manually.
+  if (typeof opts.reactServerCondition !== 'undefined') {
+    if (opts.reactServerCondition) {
+      // Alias to the `react-server` exports.
+      alias['server-only$'] = 'next/dist/compiled/server-only/empty'
+      alias['client-only$'] = 'next/dist/compiled/client-only/error'
+    } else {
+      alias['server-only$'] = 'next/dist/compiled/server-only/index'
+      alias['client-only$'] = 'next/dist/compiled/client-only/index'
+    }
+  }
+
   return alias
 }
 
@@ -1857,7 +1871,7 @@ export default async function getBaseWebpackConfig(
                     [require.resolve('next/dynamic')]: require.resolve(
                       'next/dist/shared/lib/app-dynamic'
                     ),
-                    ...createReactAliases(bundledReactChannel, {
+                    ...createRSCAliases(bundledReactChannel, {
                       reactSharedSubset: false,
                       reactDomServerRenderingStub: false,
                     }),
@@ -1888,9 +1902,10 @@ export default async function getBaseWebpackConfig(
                     // If missing the alias override here, the default alias will be used which aliases
                     // react to the direct file path, not the package name. In that case the condition
                     // will be ignored completely.
-                    ...createReactAliases(bundledReactChannel, {
+                    ...createRSCAliases(bundledReactChannel, {
                       reactSharedSubset: true,
                       reactDomServerRenderingStub: true,
+                      reactServerCondition: true,
                     }),
                   },
                 },
@@ -1954,9 +1969,10 @@ export default async function getBaseWebpackConfig(
                       // It needs `conditionNames` here to require the proper asset,
                       // when react is acting as dependency of compiled/react-dom.
                       alias: {
-                        ...createReactAliases(bundledReactChannel, {
+                        ...createRSCAliases(bundledReactChannel, {
                           reactSharedSubset: true,
                           reactDomServerRenderingStub: true,
+                          reactServerCondition: true,
                         }),
                       },
                     },
@@ -1966,9 +1982,10 @@ export default async function getBaseWebpackConfig(
                     issuerLayer: WEBPACK_LAYERS.client,
                     resolve: {
                       alias: {
-                        ...createReactAliases(bundledReactChannel, {
+                        ...createRSCAliases(bundledReactChannel, {
                           reactSharedSubset: false,
                           reactDomServerRenderingStub: true,
+                          reactServerCondition: false,
                         }),
                       },
                     },
@@ -1980,10 +1997,11 @@ export default async function getBaseWebpackConfig(
                 issuerLayer: WEBPACK_LAYERS.appClient,
                 resolve: {
                   alias: {
-                    ...createReactAliases(bundledReactChannel, {
+                    ...createRSCAliases(bundledReactChannel, {
                       // Only alias server rendering stub in client SSR layer.
                       reactSharedSubset: false,
                       reactDomServerRenderingStub: false,
+                      reactServerCondition: false,
                     }),
                   },
                 },
@@ -2180,7 +2198,7 @@ export default async function getBaseWebpackConfig(
             ]
           : []),
         {
-          test: /node_modules[/\\]client-only[/\\]error.js/,
+          test: /(node_modules|next[/\\]dist[/\\]compiled)[/\\]client-only[/\\]error.js/,
           loader: 'next-invalid-import-error-loader',
           issuerLayer: {
             or: [WEBPACK_LAYERS.server, WEBPACK_LAYERS.action],
@@ -2191,7 +2209,7 @@ export default async function getBaseWebpackConfig(
           },
         },
         {
-          test: /node_modules[/\\]server-only[/\\]index.js/,
+          test: /(node_modules|next[/\\]dist[/\\]compiled)[/\\]server-only[/\\]index.js/,
           loader: 'next-invalid-import-error-loader',
           issuerLayer: WEBPACK_LAYERS.client,
           options: {

diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
@@ -355,13 +355,21 @@ export class ClientReferenceEntryPlugin {
         }
 
         if (actionEntryImports.size > 0) {
-          if (!actionMapsPerClientEntry[name]) {
-            actionMapsPerClientEntry[name] = new Map()
+          if (!this.useServerActions) {
+            compilation.errors.push(
+              new Error(
+                'Server Actions require `experimental.serverActions` option to be enabled in your Next.js config: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions'
+              )
+            )
+          } else {
+            if (!actionMapsPerClientEntry[name]) {
+              actionMapsPerClientEntry[name] = new Map()
+            }
+            actionMapsPerClientEntry[name] = new Map([
+              ...actionMapsPerClientEntry[name],
+              ...actionEntryImports,
+            ])
           }
-          actionMapsPerClientEntry[name] = new Map([
-            ...actionMapsPerClientEntry[name],
-            ...actionEntryImports,
-          ])
         }
       })
 

diff --git a/packages/next/src/server/require-hook.ts b/packages/next/src/server/require-hook.ts
@@ -23,8 +23,6 @@ addHookAliases([
   ['styled-jsx', require.resolve('styled-jsx')],
   ['styled-jsx/style', require.resolve('styled-jsx/style')],
   ['styled-jsx/style', require.resolve('styled-jsx/style')],
-  ['server-only', require.resolve('next/dist/compiled/server-only')],
-  ['client-only', require.resolve('next/dist/compiled/client-only')],
 ])
 
 // Override built-in React packages if necessary

diff --git a/test/e2e/app-dir/actions/app-action-export.test.ts b/test/e2e/app-dir/actions/app-action-export.test.ts
@@ -6,6 +6,11 @@ createNextDescribe(
     files: __dirname,
     skipStart: true,
     skipDeployment: true,
+    dependencies: {
+      react: 'latest',
+      'react-dom': 'latest',
+      'server-only': 'latest',
+    },
   },
   ({ next, isNextStart }) => {
     if (!isNextStart) {

diff --git a/test/e2e/app-dir/actions/app-action-invalid.test.ts b/test/e2e/app-dir/actions/app-action-invalid.test.ts
@@ -0,0 +1,41 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+  'app-dir action invalid config',
+  {
+    files: __dirname,
+    skipDeployment: true,
+    dependencies: {
+      react: 'latest',
+      'react-dom': 'latest',
+      'server-only': 'latest',
+    },
+  },
+  ({ next, isNextStart }) => {
+    if (!isNextStart) {
+      it('skip test for dev mode', () => {})
+      return
+    }
+
+    beforeAll(async () => {
+      await next.stop()
+      await next.patchFile(
+        'next.config.js',
+        `
+      module.exports = {
+        experimental: {},
+      }
+      `
+      )
+      try {
+        await next.build()
+      } catch {}
+    })
+
+    it('should error if serverActions is not enabled', async () => {
+      expect(next.cliOutput).toContain(
+        'Server Actions require `experimental.serverActions` option'
+      )
+    })
+  }
+)
diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts
@@ -11,6 +11,11 @@ createNextDescribe(
   'app-dir action handling',
   {
     files: __dirname,
+    dependencies: {
+      react: 'latest',
+      'react-dom': 'latest',
+      'server-only': 'latest',
+    },
   },
   ({ next, isNextDev, isNextStart, isNextDeploy }) => {
     it('should handle basic actions correctly', async () => {

diff --git a/test/e2e/app-dir/actions/app/client/actions.js b/test/e2e/app-dir/actions/app/client/actions.js
@@ -1,5 +1,7 @@
 'use server'
 
+import 'server-only'
+
 import { redirect } from 'next/navigation'
 import { headers, cookies } from 'next/headers'