Skip to content

Commit

Permalink
Read page name from work store in server module map proxy (#71669)
Browse files Browse the repository at this point in the history
This avoids a race condition when running `next build` where the
manifest singleton might be overwritten with the next page's manifest
while the previous page is still being rendered.

With this PR, we are not storing a page-specific singleton but the whole
manifest, and only when accessing an entry is the page read from the
work store, and thus scoped to the current page. An exception is when a
server module map is needed during module evaluation when no work store
is provided, e.g. to create a server action using a higher-order
function. In this case it should be safe to return any entry from the
manifest that matches the action ID. They all refer to the same module
ID, which must also exist in the current page bundle. (This is currently
not guaranteed in Turbopack, and needs to be fixed.)
  • Loading branch information
unstubbable authored Oct 23, 2024
1 parent b258244 commit 03ff785
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 14 deletions.
1 change: 0 additions & 1 deletion packages/next/src/build/templates/edge-app-route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ if (rscManifest && rscServerManifest) {
serverActionsManifest: rscServerManifest,
serverModuleMap: createServerModuleMap({
serverActionsManifest: rscServerManifest,
pageName: 'VAR_PAGE',
}),
})
}
Expand Down
1 change: 0 additions & 1 deletion packages/next/src/build/templates/edge-ssr-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ if (rscManifest && rscServerManifest) {
serverActionsManifest: rscServerManifest,
serverModuleMap: createServerModuleMap({
serverActionsManifest: rscServerManifest,
pageName: 'VAR_PAGE',
}),
})
}
Expand Down
31 changes: 27 additions & 4 deletions packages/next/src/server/app-render/action-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,49 @@ import type { ActionManifest } from '../../build/webpack/plugins/flight-client-e
import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths'
import { pathHasPrefix } from '../../shared/lib/router/utils/path-has-prefix'
import { removePathPrefix } from '../../shared/lib/router/utils/remove-path-prefix'
import { workAsyncStorage } from './work-async-storage.external'

// This function creates a Flight-acceptable server module map proxy from our
// Server Reference Manifest similar to our client module map.
// This is because our manifest contains a lot of internal Next.js data that
// are relevant to the runtime, workers, etc. that React doesn't need to know.
export function createServerModuleMap({
serverActionsManifest,
pageName,
}: {
serverActionsManifest: ActionManifest
pageName: string
}) {
return new Proxy(
{},
{
get: (_, id: string) => {
const workerEntry =
const workers =
serverActionsManifest[
process.env.NEXT_RUNTIME === 'edge' ? 'edge' : 'node'
][id].workers[normalizeWorkerPageName(pageName)]
][id].workers

const workStore = workAsyncStorage.getStore()

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

if (workStore) {
workerEntry = workers[normalizeWorkerPageName(workStore.page)]
} else {
// If there's no work store defined, we can assume that a server
// module map is needed during module evaluation, e.g. to create a
// server action using a higher-order function. Therefore it should be
// safe to return any entry from the manifest that matches the action
// ID. They all refer to the same module ID, which must also exist in
// the current page bundle. TODO: This is currently not guaranteed in
// Turbopack, and needs to be fixed.
workerEntry = Object.values(workers).at(0)
}

if (!workerEntry) {
return undefined
}

if (typeof workerEntry === 'string') {
return { id: workerEntry, name: id, chunks: [] }
Expand Down
5 changes: 1 addition & 4 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1057,10 +1057,7 @@ async function renderToHTMLOrFlightImpl(
// TODO: fix this typescript
const clientReferenceManifest = renderOpts.clientReferenceManifest!

const serverModuleMap = createServerModuleMap({
serverActionsManifest,
pageName: renderOpts.page,
})
const serverModuleMap = createServerModuleMap({ serverActionsManifest })

setReferenceManifestsSingleton({
clientReferenceManifest,
Expand Down
7 changes: 3 additions & 4 deletions packages/next/src/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,15 @@ async function loadComponentsImpl<N = any>({
: null,
])

// Before requring the actual page module, we have to set the reference manifests
// to our global store so Server Action's encryption util can access to them
// at the top level of the page module.
// Before requiring the actual page module, we have to set the reference
// manifests to our global store so Server Action's encryption util can access
// to them at the top level of the page module.
if (serverActionsManifest && clientReferenceManifest) {
setReferenceManifestsSingleton({
clientReferenceManifest,
serverActionsManifest,
serverModuleMap: createServerModuleMap({
serverActionsManifest,
pageName: page,
}),
})
}
Expand Down

0 comments on commit 03ff785

Please sign in to comment.