turbopack: Support Actions in both RSC and Client layers (#57475)

### What?

Adds support for Server Actions imported by both server and client.

### Why?

If an Action is imported by both the Client and RSC layers, we need to
support them as if they're the same action.

### How?

First, we need to ensure both layers create the same action hash ids,
which we can then use to deduplicate actions imported by both layers. If
a conflict is found, we prefer the RSC layer's action.

Closes WEB-1879

---------

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

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

@@ -1,16 +1,15 @@
 use std::{io::Write, iter::once};
 
 use anyhow::{bail, Result};
-use indexmap::IndexMap;
-use indoc::writedoc;
+use indexmap::{map::Entry, IndexMap};
 use next_core::{
     next_manifests::{ActionLayer, ActionManifestWorkerEntry, ServerReferenceManifest},
     util::{get_asset_prefix_from_pathname, NextRuntime},
 };
 use next_swc::server_actions::parse_server_actions;
 use turbo_tasks::{
     graph::{GraphTraversal, NonDeterministic},
-    TryFlatJoinIterExt, TryJoinIterExt, Value, ValueToString, Vc,
+    TryFlatJoinIterExt, Value, ValueToString, Vc,
 };
 use turbopack_binding::{
     turbo::tasks_fs::{rope::RopeBuilder, File, FileSystemPath},
@@ -78,33 +77,33 @@ pub(crate) async fn create_server_actions_manifest(
 async fn build_server_actions_loader(
     node_root: Vc<FileSystemPath>,
     page_name: &str,
-    actions: Vc<ModuleActionMap>,
+    actions: Vc<AllActions>,
     asset_context: Vc<Box<dyn AssetContext>>,
 ) -> Result<Vc<Box<dyn EcmascriptChunkPlaceable>>> {
     let actions = actions.await?;
 
-    let mut contents = RopeBuilder::from("__turbopack_export_value__({\n");
-    let mut import_map = IndexMap::with_capacity(actions.len());
-
     // Every module which exports an action (that is accessible starting from our
     // app page entry point) will be present. We generate a single loader file
     // which lazily imports the respective module's chunk_item id and invokes
     // the exported action function.
-    for (i, (module, actions_map)) in actions.iter().enumerate() {
-        let module_name = format!("ACTIONS_MODULE{i}");
-        for (hash_id, name) in &*actions_map.await? {
-            writedoc!(
-                contents,
-                "  '{hash_id}': (...args) => (0, require('{module_name}')['{name}'])(...args),",
-            )?;
-        }
-        import_map.insert(module_name, module.1);
+    let mut contents = RopeBuilder::from("__turbopack_export_value__({\n");
+    let mut import_map = IndexMap::new();
+    for (hash_id, (_layer, name, module)) in actions.iter() {
+        let index = import_map.len();
+        let module_name = import_map
+            .entry(*module)
+            .or_insert_with(|| format!("ACTIONS_MODULE{index}"));
+        write!(
+            contents,
+            "  '{hash_id}': (...args) => (0, require('{module_name}')['{name}'])(...args),",
+        )?;
     }
     write!(contents, "}});")?;
 
     let output_path = node_root.join(format!("server/app{page_name}/actions.js"));
     let file = File::from(contents.build());
     let source = VirtualSource::new(output_path, AssetContent::file(file.into()));
+    let import_map = import_map.into_iter().map(|(k, v)| (v, k)).collect();
     let module = asset_context.process(
         Vc::upcast(source),
         Value::new(ReferenceType::Internal(Vc::cell(import_map))),
@@ -126,7 +125,7 @@ async fn build_manifest(
     pathname: &str,
     page_name: &str,
     runtime: NextRuntime,
-    actions: Vc<ModuleActionMap>,
+    actions: Vc<AllActions>,
     loader_id: Vc<String>,
 ) -> Result<Vc<Box<dyn OutputAsset>>> {
     let manifest_path_prefix = get_asset_prefix_from_pathname(pathname);
@@ -144,16 +143,13 @@ async fn build_manifest(
         NextRuntime::NodeJs => &mut manifest.node,
     };
 
-    for ((layer, _), action_map) in actions_value {
-        let action_map = action_map.await?;
-        for hash in action_map.keys() {
-            let entry = mapping.entry(hash.clone()).or_default();
-            entry.workers.insert(
-                format!("app{page_name}"),
-                ActionManifestWorkerEntry::String(loader_id_value.clone_value()),
-            );
-            entry.layer.insert(format!("app{page_name}"), *layer);
-        }
+    for (hash_id, (layer, _name, _module)) in actions_value {
+        let entry = mapping.entry(hash_id.clone()).or_default();
+        entry.workers.insert(
+            format!("app{page_name}"),
+            ActionManifestWorkerEntry::String(loader_id_value.clone_value()),
+        );
+        entry.layer.insert(format!("app{page_name}"), *layer);
     }
 
     Ok(Vc::upcast(VirtualOutputAsset::new(
@@ -175,8 +171,8 @@ async fn get_actions(
     rsc_entry: Vc<Box<dyn Module>>,
     server_reference_modules: Vc<Vec<Vc<Box<dyn Module>>>>,
     asset_context: Vc<Box<dyn AssetContext>>,
-) -> Result<Vc<ModuleActionMap>> {
-    let mut all_actions = NonDeterministic::new()
+) -> Result<Vc<AllActions>> {
+    let actions = NonDeterministic::new()
         .skip_duplicates()
         .visit(
             once((ActionLayer::Rsc, rsc_entry)).chain(
@@ -193,44 +189,61 @@ async fn get_actions(
         .into_iter()
         .map(parse_actions_filter_map)
         .try_flat_join()
-        .await?
-        .into_iter()
-        .map(|((layer, module), actions)| async move {
-            let module = if layer == ActionLayer::Rsc {
-                module
-            } else {
-                // The ActionBrowser layer's module is in the Client context, and we need to
-                // bring it into the RSC context.
-                let source = VirtualSource::new_with_ident(
-                    module.ident().with_modifier(action_modifier()),
-                    module.content(),
-                );
-                let ty = if let Some(module) =
-                    Vc::try_resolve_downcast_type::<EcmascriptModuleAsset>(module).await?
-                {
-                    if module.await?.ty == EcmascriptModuleAssetType::Ecmascript {
-                        ReferenceType::EcmaScriptModules(
-                            EcmaScriptModulesReferenceSubType::Undefined,
-                        )
-                    } else {
-                        ReferenceType::TypeScript(TypeScriptReferenceSubType::Undefined)
+        .await?;
+
+    // Actions can be imported by both Client and RSC layers, in which case we need
+    // to use the RSC layer's module. We do that by merging the hashes (which match
+    // in both layers) and preferring the RSC layer's action.
+    let mut all_actions: HashToLayerNameModule = IndexMap::new();
+    for ((layer, module), actions_map) in actions.iter() {
+        let module = if *layer == ActionLayer::Rsc {
+            *module
+        } else {
+            to_rsc_context(*module, asset_context).await?
+        };
+
+        for (hash_id, name) in &*actions_map.await? {
+            match all_actions.entry(hash_id.to_owned()) {
+                Entry::Occupied(e) => {
+                    if e.get().0 == ActionLayer::ActionBrowser {
+                        *e.into_mut() = (*layer, name.to_string(), module);
                     }
-                } else {
-                    ReferenceType::TypeScript(TypeScriptReferenceSubType::Undefined)
-                };
-                asset_context.process(Vc::upcast(source), Value::new(ty))
-            };
-            Ok(((layer, module), actions))
-        })
-        .try_join()
-        .await?
-        .into_iter()
-        .collect::<IndexMap<_, _>>();
+                }
+                Entry::Vacant(e) => {
+                    e.insert((*layer, name.to_string(), module));
+                }
+            }
+        }
+    }
 
     all_actions.sort_keys();
     Ok(Vc::cell(all_actions))
 }
 
+/// The ActionBrowser layer's module is in the Client context, and we need to
+/// bring it into the RSC context.
+async fn to_rsc_context(
+    module: Vc<Box<dyn Module>>,
+    asset_context: Vc<Box<dyn AssetContext>>,
+) -> Result<Vc<Box<dyn Module>>> {
+    let source = VirtualSource::new_with_ident(
+        module.ident().with_modifier(action_modifier()),
+        module.content(),
+    );
+    let ty = if let Some(module) =
+        Vc::try_resolve_downcast_type::<EcmascriptModuleAsset>(module).await?
+    {
+        if module.await?.ty == EcmascriptModuleAssetType::Ecmascript {
+            ReferenceType::EcmaScriptModules(EcmaScriptModulesReferenceSubType::Undefined)
+        } else {
+            ReferenceType::TypeScript(TypeScriptReferenceSubType::Undefined)
+        }
+    } else {
+        ReferenceType::TypeScript(TypeScriptReferenceSubType::Undefined)
+    };
+    Ok(asset_context.process(Vc::upcast(source), Value::new(ty)))
+}
+
 /// Our graph traversal visitor, which finds the primary modules directly
 /// referenced by [parent].
 async fn get_referenced_modules(
@@ -280,15 +293,15 @@ async fn parse_actions_filter_map(
     })
 }
 
-type LayerModuleActionMap = IndexMap<(ActionLayer, Vc<Box<dyn Module>>), Vc<ActionMap>>;
+type HashToLayerNameModule = IndexMap<String, (ActionLayer, String, Vc<Box<dyn Module>>)>;
 
 /// A mapping of every module which exports a Server Action, with the hashed id
 /// and exported name of each found action.
 #[turbo_tasks::value(transparent)]
-struct ModuleActionMap(LayerModuleActionMap);
+struct AllActions(HashToLayerNameModule);
 
 #[turbo_tasks::value_impl]
-impl ModuleActionMap {
+impl AllActions {
     #[turbo_tasks::function]
     pub fn empty() -> Vc<Self> {
         Vc::cell(IndexMap::new())

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

@@ -40,9 +40,7 @@ struct NextServerActions {
 impl CustomTransformer for NextServerActions {
     async fn transform(&self, program: &mut Program, ctx: &TransformContext<'_>) -> Result<()> {
         let mut actions = server_actions(
-            // The same action can be imported by both server and client, and we need to give both
-            // types a distinct Action hash ID.
-            &FileName::Real(format!("{}_{:?}", ctx.file_path_str, self.transform).into()),
+            &FileName::Real(ctx.file_path_str.into()),
             Config {
                 is_react_server_layer: matches!(self.transform, ActionsTransform::Server),
                 enabled: true,

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

@@ -19,7 +19,7 @@ createNextDescribe(
       'server-only': 'latest',
     },
   },
-  ({ next, isNextDev, isNextStart, isNextDeploy }) => {
+  ({ next, isNextDev, isNextStart, isNextDeploy, isTurbopack }) => {
     it('should handle basic actions correctly', async () => {
       const browser = await next.browser('/server')
 
@@ -389,7 +389,24 @@ createNextDescribe(
         const pageBundle = await fs.readFile(
           join(next.testDir, '.next', 'server', 'app', 'client', 'page.js')
         )
-        expect(pageBundle.toString()).toContain('node_modules/nanoid/index.js')
+        if (isTurbopack) {
+          const chunkPaths = pageBundle
+            .toString()
+            .matchAll(/loadChunk\("([^"]*)"\)/g)
+          // @ts-ignore
+          const reads = [...chunkPaths].map(async (match) => {
+            const bundle = await fs.readFile(
+              join(next.testDir, '.next', ...match[1].split(/[\\/]/g))
+            )
+            return bundle.toString().includes('node_modules/nanoid/index.js')
+          })
+
+          expect(await Promise.all(reads)).toContain(true)
+        } else {
+          expect(pageBundle.toString()).toContain(
+            'node_modules/nanoid/index.js'
+          )
+        }
       })
     }
 

test/turbopack-tests-manifest.json

@@ -2206,6 +2206,7 @@
       "app-dir action handling fetch actions should revalidate when cookies.set is called",
       "app-dir action handling fetch actions should revalidate when cookies.set is called in a client action",
       "app-dir action handling fetch actions should store revalidation data in the prefetch cache",
+      "app-dir action handling should bundle external libraries if they are on the action layer",
       "app-dir action handling should handle basic actions correctly",
       "app-dir action handling should not block navigation events while a server action is in flight",
       "app-dir action handling should only submit action once when resubmitting an action after navigation",
@@ -2217,16 +2218,14 @@
       "app-dir action handling should support headers in client imported actions",
       "app-dir action handling should support hoc auth wrappers",
       "app-dir action handling should support importing actions in client components",
+      "app-dir action handling should support importing the same action module instance in both server and action layers",
       "app-dir action handling should support next/dynamic with ssr: false",
       "app-dir action handling should support notFound",
       "app-dir action handling should support notFound (javascript disabled)",
       "app-dir action handling should support setting cookies in route handlers with the correct overrides",
       "app-dir action handling should support uploading files"
     ],
-    "failed": [
-      "app-dir action handling should bundle external libraries if they are on the action layer",
-      "app-dir action handling should support importing the same action module instance in both server and action layers"
-    ],
+    "failed": [],
     "pending": [
       "app-dir action handling fetch actions should handle revalidateTag + redirect"
     ],