Avoid server action function indirection in Turbopack

As a follow-up to vercel#71572, this PR replaces the server action function
indirection in Turbopack's generated action loader module with a direct
re-export of the actions, using the action ID as export name.

The server action identity test that was added in vercel#71572 was not
sufficient to uncover that Turbopack was still using the wrapper
functions. To fix that, we've added another test here, which uses a
combination of `"use server"` and `"use cache"` functions in a page.
This test does fail in Turbopack without the loader changes.

In addition, the building of the server reference manifest is adjusted
to account for the new structure that expects `{moduleId: string, async:
boolean}` instead of `string`. The `async` flag is set based on the
loader's chunk item. I believe this is currently always `false` though.
However, Turbopack can still resolve the modules just fine, even when
there are outgoing async connections.

Author: unstubbable

crates/next-api/src/server_actions.rs

@@ -3,7 +3,9 @@ use std::{collections::BTreeMap, io::Write, iter::once};
 use anyhow::{bail, Context, Result};
 use indexmap::map::Entry;
 use next_core::{
-    next_manifests::{ActionLayer, ActionManifestWorkerEntry, ServerReferenceManifest},
+    next_manifests::{
+        ActionLayer, ActionManifestModuleId, ActionManifestWorkerEntry, ServerReferenceManifest,
+    },
     util::NextRuntime,
 };
 use swc_core::{
@@ -22,7 +24,7 @@ use turbo_tasks::{
 use turbo_tasks_fs::{self, rope::RopeBuilder, File, FileSystemPath};
 use turbopack_core::{
     asset::AssetContent,
-    chunk::{ChunkItemExt, ChunkableModule, ChunkingContext, EvaluatableAsset},
+    chunk::{ChunkItem, ChunkItemExt, ChunkableModule, ChunkingContext, EvaluatableAsset},
     context::AssetContext,
     file_source::FileSource,
     module::{Module, Modules},
@@ -69,11 +71,8 @@ pub(crate) async fn create_server_actions_manifest(
         .await?
         .context("loader module must be evaluatable")?;
 
-    let loader_id = loader
-        .as_chunk_item(Vc::upcast(chunking_context))
-        .id()
-        .to_string();
-    let manifest = build_manifest(node_root, page_name, runtime, actions, loader_id).await?;
+    let chunk_item = loader.as_chunk_item(Vc::upcast(chunking_context));
+    let manifest = build_manifest(node_root, page_name, runtime, actions, chunk_item).await?;
     Ok(ServerActionsManifest {
         loader: evaluable,
         manifest,
@@ -96,11 +95,11 @@ async fn build_server_actions_loader(
 ) -> Result<Vc<Box<dyn EcmascriptChunkPlaceable>>> {
     let actions = actions.await?;
 
-    // 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.
-    let mut contents = RopeBuilder::from("__turbopack_export_value__({\n");
+    // 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 re-exports the respective module's action function using the
+    // hashed ID as export name.
+    let mut contents = RopeBuilder::from("");
     let mut import_map = FxIndexMap::default();
     for (hash_id, (_layer, name, module)) in actions.iter() {
         let index = import_map.len();
@@ -109,11 +108,9 @@ async fn build_server_actions_loader(
             .or_insert_with(|| format!("ACTIONS_MODULE{index}").into());
         writeln!(
             contents,
-            "  '{hash_id}': (...args) => Promise.resolve(require('{module_name}')).then(mod => \
-             (0, mod['{name}'])(...args)),",
+            "export {{{name} as '{hash_id}'}} from '{module_name}'"
         )?;
     }
-    write!(contents, "}});")?;
 
     let output_path =
         project_path.join(format!(".next-internal/server/app{page_name}/actions.js").into());
@@ -143,7 +140,7 @@ async fn build_manifest(
     page_name: RcStr,
     runtime: NextRuntime,
     actions: Vc<AllActions>,
-    loader_id: Vc<RcStr>,
+    chunk_item: Vc<Box<dyn ChunkItem>>,
 ) -> Result<Vc<Box<dyn OutputAsset>>> {
     let manifest_path_prefix = &page_name;
     let manifest_path = node_root
@@ -155,7 +152,7 @@ async fn build_manifest(
     let key = format!("app{page_name}");
 
     let actions_value = actions.await?;
-    let loader_id_value = loader_id.await?;
+    let loader_id = chunk_item.id().to_string().await?;
     let mapping = match runtime {
         NextRuntime::Edge => &mut manifest.edge,
         NextRuntime::NodeJs => &mut manifest.node,
@@ -165,7 +162,10 @@ async fn build_manifest(
         let entry = mapping.entry(hash_id.as_str()).or_default();
         entry.workers.insert(
             &key,
-            ActionManifestWorkerEntry::String(loader_id_value.as_str()),
+            ActionManifestWorkerEntry {
+                module_id: ActionManifestModuleId::String(loader_id.as_str()),
+                is_async: *chunk_item.is_self_async().await?,
+            },
         );
         entry.layer.insert(&key, *layer);
     }

crates/next-core/src/next_manifests/mod.rs

@@ -194,9 +194,16 @@ pub struct ActionManifestEntry<'a> {
 }
 
 #[derive(Serialize, Debug)]
-#[serde(rename_all = "camelCase")]
+pub struct ActionManifestWorkerEntry<'a> {
+    #[serde(rename = "moduleId")]
+    pub module_id: ActionManifestModuleId<'a>,
+    #[serde(rename = "async")]
+    pub is_async: bool,
+}
+
+#[derive(Serialize, Debug)]
 #[serde(untagged)]
-pub enum ActionManifestWorkerEntry<'a> {
+pub enum ActionManifestModuleId<'a> {
     String(&'a str),
     Number(f64),
 }

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

@@ -55,11 +55,7 @@ const PLUGIN_NAME = 'FlightClientEntryPlugin'
 type Actions = {
   [actionId: string]: {
     workers: {
-      [name: string]:
-        | { moduleId: string | number; async: boolean }
-        // TODO: This is legacy for Turbopack, and needs to be changed to the
-        // object above.
-        | string
+      [name: string]: { moduleId: string | number; async: boolean }
     }
     // Record which layer the action is in (rsc or sc_action), in the specific entry.
     layer: {

packages/next/src/server/app-render/action-utils.ts

@@ -26,7 +26,6 @@ export function createServerModuleMap({
 
         let workerEntry:
           | { moduleId: string | number; async: boolean }
-          | string
           | undefined
 
         if (workStore) {
@@ -46,10 +45,6 @@ export function createServerModuleMap({
           return undefined
         }
 
-        if (typeof workerEntry === 'string') {
-          return { id: workerEntry, name: id, chunks: [] }
-        }
-
         const { moduleId, async } = workerEntry
 
         return { id: moduleId, name: id, chunks: [], async }

packages/next/src/server/use-cache/use-cache-wrapper.ts

@@ -679,7 +679,7 @@ export function cache(kind: string, id: string, fn: any) {
         moduleMap: isEdgeRuntime
           ? clientReferenceManifest.edgeRscModuleMapping
           : clientReferenceManifest.rscModuleMapping,
-        serverModuleMap: null,
+        serverModuleMap: getServerModuleMap(),
       }
 
       return createFromReadableStream(stream, {

test/e2e/app-dir/use-cache/app/with-server-action/form.tsx

@@ -0,0 +1,22 @@
+'use client'
+
+import { ReactNode } from 'react'
+import { useActionState } from 'react'
+
+export function Form({
+  action,
+  children,
+}: {
+  action: () => Promise<string>
+  children: ReactNode
+}) {
+  const [result, formAction] = useActionState(action, 'initial')
+
+  return (
+    <form action={formAction}>
+      <button>Submit</button>
+      <p>{result}</p>
+      {children}
+    </form>
+  )
+}

test/e2e/app-dir/use-cache/app/with-server-action/page.tsx

@@ -0,0 +1,17 @@
+import { Form } from './form'
+
+async function action() {
+  'use server'
+
+  return 'result'
+}
+
+export default async function Page() {
+  'use cache'
+
+  return (
+    <Form action={action}>
+      <p>{Date.now()}</p>
+    </Form>
+  )
+}

test/e2e/app-dir/use-cache/use-cache.test.ts

@@ -231,4 +231,19 @@ describe('use-cache', () => {
       expect(meta.headers['x-next-cache-tags']).toContain('a,c,b')
     })
   }
+
+  // TODO: There's a bug with the server actions manifest singleton during next
+  // build that would make this test flaky. Enable the test for isNextStart when
+  // the singleton has been replaced with a proper solution.
+  if (!isNextStart) {
+    it('can reference server actions in "use cache" functions', async () => {
+      const browser = await next.browser('/with-server-action')
+      expect(await browser.elementByCss('p').text()).toBe('initial')
+      await browser.elementByCss('button').click()
+
+      await retry(async () => {
+        expect(await browser.elementByCss('p').text()).toBe('result')
+      })
+    })
+  }
 })