Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid server action function indirection in Turbopack #71628

Merged
merged 3 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions crates/next-api/src/server_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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},
Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand All @@ -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());
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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);
}
Expand Down
11 changes: 9 additions & 2 deletions crates/next-core/src/next_manifests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
5 changes: 0 additions & 5 deletions packages/next/src/server/app-render/action-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export function createServerModuleMap({

let workerEntry:
| { moduleId: string | number; async: boolean }
| string
| undefined

if (workStore) {
Expand All @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/use-cache/use-cache-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
22 changes: 22 additions & 0 deletions test/e2e/app-dir/use-cache/app/with-server-action/form.tsx
Original file line number Diff line number Diff line change
@@ -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>
)
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/use-cache/app/with-server-action/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { connection } from 'next/server'

export default async function DynamicLayout({
children,
}: {
children: React.ReactNode
}) {
// TODO: This is a workaround for Turbopack. Figure out why this fails during
// prerendering with:
// TypeError: Cannot read properties of undefined (reading 'Form')
Comment on lines +8 to +10
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cause for this error is a race condition for setting the client manifest into the singleton, similar to what we fixed for the server manifest in #71669. Needs to be fixed in a follow-up.

await connection()

return children
}
17 changes: 17 additions & 0 deletions test/e2e/app-dir/use-cache/app/with-server-action/page.tsx
Original file line number Diff line number Diff line change
@@ -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>
)
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/use-cache/use-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,14 @@ describe('use-cache', () => {
expect(meta.headers['x-next-cache-tags']).toContain('a,c,b')
})
}

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')
})
})
})
Loading