Change the Server Actions feature flag to be validated at compile time (

#52147)

Currently we are validating the `experimental.serverActions` flag when creating the actual entries for Server Actions, this causes two problems. One is that syntax errors caught at compilation time are still shown, even if you don't have this flag enabled. Another problem is we still traverse the client graph to collect these action modules even if the flag isn't enabled.

This PR moves that check to be happening at compilation time, which addresses the two above but also brings the extra benefit of showing the exact span and module trace that errors:

<img width="974" alt="CleanShot 2023-07-03 at 20 26 34@2x" src="https://github.com/vercel/next.js/assets/3676859/1676b1f6-e205-4963-bce4-5b515a698e9c">

packages/next-swc/crates/core/src/server_actions.rs

@@ -22,6 +22,7 @@ use turbopack_binding::swc::core::{
 #[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct Config {
     pub is_server: bool,
+    pub enabled: bool
 }
 
 pub fn server_actions<C: Comments>(
@@ -101,6 +102,7 @@ impl<C: Comments> ServerActions<C> {
                     &mut body.stmts,
                     remove_directive,
                     &mut is_action_fn,
+                    self.config.enabled,
                 );
 
                 if is_action_fn && !self.config.is_server {
@@ -721,6 +723,7 @@ impl<C: Comments> VisitMut for ServerActions<C> {
             stmts,
             &mut self.in_action_file,
             &mut self.has_action,
+            self.config.enabled,
         );
 
         let old_annotations = self.annotations.take();
@@ -1231,6 +1234,7 @@ fn remove_server_directive_index_in_module(
     stmts: &mut Vec<ModuleItem>,
     in_action_file: &mut bool,
     has_action: &mut bool,
+    enabled: bool,
 ) {
     let mut is_directive = true;
 
@@ -1244,6 +1248,16 @@ fn remove_server_directive_index_in_module(
                     if is_directive {
                         *in_action_file = true;
                         *has_action = true;
+                        if !enabled {
+                            HANDLER.with(|handler| {
+                                handler
+                                    .struct_span_err(
+                                        *span,
+                                        "To use Server Actions, please enable the feature flag in your Next.js config. Read more: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#convention",
+                                    )
+                                    .emit()
+                            });
+                        }
                         return false;
                     } else {
                         HANDLER.with(|handler| {
@@ -1320,6 +1334,7 @@ fn remove_server_directive_index_in_fn(
     stmts: &mut Vec<Stmt>,
     remove_directive: bool,
     is_action_fn: &mut bool,
+    enabled: bool,
 ) {
     let mut is_directive = true;
 
@@ -1332,6 +1347,16 @@ fn remove_server_directive_index_in_fn(
             if value == "use server" {
                 if is_directive {
                     *is_action_fn = true;
+                    if !enabled {
+                        HANDLER.with(|handler| {
+                            handler
+                                .struct_span_err(
+                                    *span,
+                                    "To use Server Actions, please enable the feature flag in your Next.js config. Read more: https://nextjs.org/docs/app/building-your-application/data-fetching/server-actions#convention",
+                                )
+                                .emit()
+                        });
+                    }
                     if remove_directive {
                         return false;
                     }

packages/next/src/build/swc/options.ts

@@ -43,6 +43,7 @@ function getBaseSWCOptions({
   swcCacheDir,
   isServerLayer,
   hasServerComponents,
+  isServerActionsEnabled,
 }: {
   filename: string
   jest?: boolean
@@ -57,6 +58,7 @@ function getBaseSWCOptions({
   swcCacheDir?: string
   isServerLayer?: boolean
   hasServerComponents?: boolean
+  isServerActionsEnabled?: boolean
 }) {
   const parserConfig = getParserOptions({ filename, jsConfig })
   const paths = jsConfig?.compilerOptions?.paths
@@ -157,6 +159,8 @@ function getBaseSWCOptions({
       : undefined,
     serverActions: hasServerComponents
       ? {
+          // TODO-APP: When Server Actions is stable, we need to remove this flag.
+          enabled: !!isServerActionsEnabled,
           isServer: !!isServerLayer,
         }
       : undefined,
@@ -285,6 +289,7 @@ export function getLoaderSWCOptions({
   relativeFilePathFromRoot,
   hasServerComponents,
   isServerLayer,
+  isServerActionsEnabled,
 }: // This is not passed yet as "paths" resolving is handled by webpack currently.
 // resolvedBaseUrl,
 {
@@ -304,6 +309,7 @@ export function getLoaderSWCOptions({
   relativeFilePathFromRoot: string
   hasServerComponents?: boolean
   isServerLayer: boolean
+  isServerActionsEnabled?: boolean
 }) {
   let baseOptions: any = getBaseSWCOptions({
     filename,
@@ -318,6 +324,7 @@ export function getLoaderSWCOptions({
     swcCacheDir,
     hasServerComponents,
     isServerLayer,
+    isServerActionsEnabled,
   })
   baseOptions.fontLoaders = {
     fontLoaders: [

packages/next/src/build/webpack/loaders/next-swc-loader.ts

@@ -73,6 +73,7 @@ async function loaderTransform(
     swcCacheDir,
     relativeFilePathFromRoot,
     hasServerComponents,
+    isServerActionsEnabled: nextConfig?.experimental?.serverActions,
     isServerLayer,
   })
 

packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts

@@ -350,21 +350,13 @@ export class FlightClientEntryPlugin {
       )
 
       if (actionEntryImports.size > 0) {
-        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 (!actionMapsPerEntry[name]) {
-            actionMapsPerEntry[name] = new Map()
-          }
-          actionMapsPerEntry[name] = new Map([
-            ...actionMapsPerEntry[name],
-            ...actionEntryImports,
-          ])
+        if (!actionMapsPerEntry[name]) {
+          actionMapsPerEntry[name] = new Map()
         }
+        actionMapsPerEntry[name] = new Map([
+          ...actionMapsPerEntry[name],
+          ...actionEntryImports,
+        ])
       }
     })
 
@@ -388,31 +380,28 @@ export class FlightClientEntryPlugin {
       )
     }
 
-    compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, () => {
-      const addedClientActionEntryList: Promise<any>[] = []
-      const actionMapsPerClientEntry: Record<string, Map<string, string[]>> = {}
-
-      // We need to create extra action entries that are created from the
-      // client layer.
-      // Start from each entry's created SSR dependency from our previous step.
-      for (const [name, ssrEntryDepdendencies] of Object.entries(
-        createdSSRDependenciesForEntry
-      )) {
-        // Collect from all entries, e.g. layout.js, page.js, loading.js, ...
-        // add agregate them.
-        const actionEntryImports = this.collectClientActionsFromDependencies({
-          compilation,
-          dependencies: ssrEntryDepdendencies,
-        })
+    if (this.useServerActions) {
+      compilation.hooks.finishModules.tapPromise(PLUGIN_NAME, () => {
+        const addedClientActionEntryList: Promise<any>[] = []
+        const actionMapsPerClientEntry: Record<
+          string,
+          Map<string, string[]>
+        > = {}
+
+        // We need to create extra action entries that are created from the
+        // client layer.
+        // Start from each entry's created SSR dependency from our previous step.
+        for (const [name, ssrEntryDepdendencies] of Object.entries(
+          createdSSRDependenciesForEntry
+        )) {
+          // Collect from all entries, e.g. layout.js, page.js, loading.js, ...
+          // add agregate them.
+          const actionEntryImports = this.collectClientActionsFromDependencies({
+            compilation,
+            dependencies: ssrEntryDepdendencies,
+          })
 
-        if (actionEntryImports.size > 0) {
-          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 (actionEntryImports.size > 0) {
             if (!actionMapsPerClientEntry[name]) {
               actionMapsPerClientEntry[name] = new Map()
             }
@@ -422,47 +411,47 @@ export class FlightClientEntryPlugin {
             ])
           }
         }
-      }
 
-      for (const [name, actionEntryImports] of Object.entries(
-        actionMapsPerClientEntry
-      )) {
-        // If an action method is already created in the server layer, we don't
-        // need to create it again in the action layer.
-        // This is to avoid duplicate action instances and make sure the module
-        // state is shared.
-        let remainingClientImportedActions = false
-        const remainingActionEntryImports = new Map<string, string[]>()
-        for (const [dep, actionNames] of actionEntryImports) {
-          const remainingActionNames = []
-          for (const actionName of actionNames) {
-            const id = name + '@' + dep + '@' + actionName
-            if (!createdActions.has(id)) {
-              remainingActionNames.push(actionName)
+        for (const [name, actionEntryImports] of Object.entries(
+          actionMapsPerClientEntry
+        )) {
+          // If an action method is already created in the server layer, we don't
+          // need to create it again in the action layer.
+          // This is to avoid duplicate action instances and make sure the module
+          // state is shared.
+          let remainingClientImportedActions = false
+          const remainingActionEntryImports = new Map<string, string[]>()
+          for (const [dep, actionNames] of actionEntryImports) {
+            const remainingActionNames = []
+            for (const actionName of actionNames) {
+              const id = name + '@' + dep + '@' + actionName
+              if (!createdActions.has(id)) {
+                remainingActionNames.push(actionName)
+              }
+            }
+            if (remainingActionNames.length > 0) {
+              remainingActionEntryImports.set(dep, remainingActionNames)
+              remainingClientImportedActions = true
             }
           }
-          if (remainingActionNames.length > 0) {
-            remainingActionEntryImports.set(dep, remainingActionNames)
-            remainingClientImportedActions = true
-          }
-        }
 
-        if (remainingClientImportedActions) {
-          addedClientActionEntryList.push(
-            this.injectActionEntry({
-              compiler,
-              compilation,
-              actions: remainingActionEntryImports,
-              entryName: name,
-              bundlePath: name,
-              fromClient: true,
-            })
-          )
+          if (remainingClientImportedActions) {
+            addedClientActionEntryList.push(
+              this.injectActionEntry({
+                compiler,
+                compilation,
+                actions: remainingActionEntryImports,
+                entryName: name,
+                bundlePath: name,
+                fromClient: true,
+              })
+            )
+          }
         }
-      }
 
-      return Promise.all(addedClientActionEntryList)
-    })
+        return Promise.all(addedClientActionEntryList)
+      })
+    }
 
     // Invalidate in development to trigger recompilation
     const invalidator = getInvalidator(compiler.outputPath)

test/e2e/app-dir/actions/app-action-invalid.test.ts

@@ -34,7 +34,7 @@ createNextDescribe(
 
     it('should error if serverActions is not enabled', async () => {
       expect(next.cliOutput).toContain(
-        'Server Actions require `experimental.serverActions` option'
+        'To use Server Actions, please enable the feature flag in your Next.js config.'
       )
     })
   }