Skip to content

Commit

Permalink
Avoid server action endpoint function indirection (#71572)
Browse files Browse the repository at this point in the history
This PR removes the extra wrapper functions and `.bind()` calls from the
action entrypoint loader. Instead, the loader module now only re-exports
the actions from the original module using the IDs as export names. This
ensures that the same action instance will always remain identical. It
also unblocks the combined usage of `"use cache"` and `"use server"`
functions.

---------

Co-authored-by: Shu Ding <[email protected]>
  • Loading branch information
unstubbable and shuding authored Oct 21, 2024
1 parent 45a328a commit 475bdb1
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,12 @@ function nextFlightActionEntryLoader(this: any) {
.flat()

return `
const actions = {
${individualActions
.map(([id, path, name]) => {
return `'${id}': () => import(/* webpackMode: "eager" */ ${JSON.stringify(
path
)}).then(mod => mod[${JSON.stringify(name)}]),`
// Re-export the same functions from the original module path as action IDs.
return `export { ${name} as "${id}" } from ${JSON.stringify(path)}`
})
.join('\n')}
}
async function endpoint(id, ...args) {
const action = await actions[id]()
return action.apply(null, args)
}
// Using CJS to avoid this to be tree-shaken away due to unused exports.
module.exports = {
${individualActions
.map(([id]) => {
return ` '${id}': endpoint.bind(null, '${id}'),`
})
.join('\n')}
}
`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ const PLUGIN_NAME = 'FlightClientEntryPlugin'
type Actions = {
[actionId: string]: {
workers: {
[name: string]: string | number
[name: string]:
| { moduleId: string | number; async: boolean }
// TODO: This is legacy for Turbopack, and needs to be changed to the
// object above.
| string
}
// Record which layer the action is in (rsc or sc_action), in the specific entry.
layer: {
Expand All @@ -79,15 +83,15 @@ const pluginState = getProxiedPluginState({
actionModServerId: {} as Record<
string,
{
server?: string | number
client?: string | number
server?: { moduleId: string | number; async: boolean }
client?: { moduleId: string | number; async: boolean }
}
>,
actionModEdgeServerId: {} as Record<
string,
{
server?: string | number
client?: string | number
server?: { moduleId: string | number; async: boolean }
client?: { moduleId: string | number; async: boolean }
}
>,

Expand Down Expand Up @@ -879,7 +883,11 @@ export class FlightClientEntryPlugin {
layer: {},
}
}
currentCompilerServerActions[id].workers[bundlePath] = ''
currentCompilerServerActions[id].workers[bundlePath] = {
moduleId: '', // TODO: What's the meaning of this?
async: false,
}

currentCompilerServerActions[id].layer[bundlePath] = fromClient
? WEBPACK_LAYERS.actionBrowser
: WEBPACK_LAYERS.reactServerComponents
Expand Down Expand Up @@ -928,6 +936,13 @@ export class FlightClientEntryPlugin {
}

compilation.hooks.succeedEntry.call(dependency, options, module)

compilation.moduleGraph
.getExportsInfo(module)
.setUsedInUnknownWay(
this.isEdgeServer ? EDGE_RUNTIME_WEBPACK : DEFAULT_RUNTIME_WEBPACK
)

return resolve(module)
}
)
Expand Down Expand Up @@ -958,7 +973,10 @@ export class FlightClientEntryPlugin {
if (!mapping[chunkGroup.name]) {
mapping[chunkGroup.name] = {}
}
mapping[chunkGroup.name][fromClient ? 'client' : 'server'] = modId
mapping[chunkGroup.name][fromClient ? 'client' : 'server'] = {
moduleId: modId,
async: compilation.moduleGraph.isAsync(mod),
}
}
})

Expand Down
15 changes: 10 additions & 5 deletions packages/next/src/server/app-render/action-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@ export function createServerModuleMap({
{},
{
get: (_, id: string) => {
return {
id: serverActionsManifest[
const workerEntry =
serverActionsManifest[
process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node'
][id].workers[normalizeWorkerPageName(pageName)],
name: id,
chunks: [],
][id].workers[normalizeWorkerPageName(pageName)]

if (typeof workerEntry === 'string') {
return { id: workerEntry, name: id, chunks: [] }
}

const { moduleId, async } = workerEntry

return { id: moduleId, name: id, chunks: [], async }
},
}
)
Expand Down
18 changes: 18 additions & 0 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,24 @@ describe('app-dir action handling', () => {
).toBe(true)
})

// we don't have access to runtime logs on deploy
if (!isNextDeploy) {
it('should keep action instances identical', async () => {
const logs: string[] = []
next.on('stdout', (log) => {
logs.push(log)
})

const browser = await next.browser('/identity')

await browser.elementByCss('button').click()

await retry(() => {
expect(logs.join('')).toContain('result: true')
})
})
}

it.each(['node', 'edge'])(
'should forward action request to a worker that contains the action handler (%s)',
async (runtime) => {
Expand Down
13 changes: 13 additions & 0 deletions test/e2e/app-dir/actions/app/identity/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use client'

export function Client({ foo, b }) {
return (
<button
onClick={() => {
foo(b)
}}
>
Trigger
</button>
)
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/actions/app/identity/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { Client } from './client'

export default async function Page() {
async function b() {
'use server'
}

async function foo(a) {
'use server'
console.log('result:', a === b)
}

return <Client foo={foo} b={b} />
}

0 comments on commit 475bdb1

Please sign in to comment.