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.

Sign up for GitHub

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

Refactor: scope requestStore to dynamic renders only #72312

Merged
merged 1 commit into from
Nov 7, 2024
+79 −80
Merged

Refactor: scope requestStore to dynamic renders only #72312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions packages/next/src/server/app-render/action-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { isNodeNextRequest, isWebNextRequest } from '../base-http/helpers'
import { RedirectStatusCode } from '../../client/components/redirect-status-code'
import { synchronizeMutableCookies } from '../async-storage/request-store'
import type { TemporaryReferenceSet } from 'react-server-dom-webpack/server.edge'
import { workUnitAsyncStorage } from '../app-render/work-unit-async-storage.external'

function formDataFromSearchQueryString(query: string) {
const searchParams = new URLSearchParams(query)
Expand Down Expand Up @@ -561,7 +562,7 @@ export async function handleAction({

return {
type: 'done',
result: await finalizeAndGenerateFlight(req, ctx, {
result: await finalizeAndGenerateFlight(req, ctx, requestStore, {
actionResult: promise,
// if the page was not revalidated, we can skip the rendering the flight tree
skipFlight: !workStore.pathWasRevalidated,
Expand Down Expand Up @@ -878,7 +879,9 @@ export async function handleAction({
actionId!
]

const returnVal = await actionHandler.apply(null, boundActionArguments)
const returnVal = await workUnitAsyncStorage.run(requestStore, () =>
actionHandler.apply(null, boundActionArguments)
)

// For form actions, we need to continue rendering the page.
if (isFetchAction) {
Expand All @@ -887,7 +890,7 @@ export async function handleAction({
requestStore,
})

actionResult = await finalizeAndGenerateFlight(req, ctx, {
actionResult = await finalizeAndGenerateFlight(req, ctx, requestStore, {
actionResult: Promise.resolve(returnVal),
// if the page was not revalidated, or if the action was forwarded from another worker, we can skip the rendering the flight tree
skipFlight: !workStore.pathWasRevalidated || actionWasForwarded,
Expand Down Expand Up @@ -963,7 +966,7 @@ export async function handleAction({
}
return {
type: 'done',
result: await finalizeAndGenerateFlight(req, ctx, {
result: await finalizeAndGenerateFlight(req, ctx, requestStore, {
skipFlight: false,
actionResult: promise,
temporaryReferences,
Expand Down Expand Up @@ -998,7 +1001,7 @@ export async function handleAction({
requestStore.phase = 'render'
return {
type: 'done',
result: await generateFlight(req, ctx, {
result: await generateFlight(req, ctx, requestStore, {
actionResult: promise,
// if the page was not revalidated, or if the action was forwarded from another worker, we can skip the rendering the flight tree
skipFlight: !workStore.pathWasRevalidated || actionWasForwarded,
Expand Down
142 changes: 69 additions & 73 deletions packages/next/src/server/app-render/app-render.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ function createErrorContext(
async function generateDynamicFlightRenderResult(
req: BaseNextRequest,
ctx: AppRenderContext,
requestStore: RequestStore,
options?: {
actionResult: ActionResult
skipFlight: boolean
Expand All @@ -534,7 +535,12 @@ async function generateDynamicFlightRenderResult(
const RSCPayload: RSCPayload & {
/** Only available during dynamicIO development builds. Used for logging errors. */
_validation?: Promise<React.ReactNode>
} = await generateDynamicRSCPayload(ctx, options)
} = await workUnitAsyncStorage.run(
requestStore,
generateDynamicRSCPayload,
ctx,
options
)

if (
// We only want this behavior when running `next dev`
Expand All @@ -559,7 +565,9 @@ async function generateDynamicFlightRenderResult(

// For app dir, use the bundled version of Flight server renderer (renderToReadableStream)
// which contains the subset React.
const flightReadableStream = ctx.componentMod.renderToReadableStream(
const flightReadableStream = workUnitAsyncStorage.run(
requestStore,
ctx.componentMod.renderToReadableStream,
RSCPayload,
ctx.clientReferenceManifest.clientModules,
{
Expand All @@ -583,13 +591,7 @@ async function generateDynamicFlightRenderResult(
async function warmupDevRender(
req: BaseNextRequest,
ctx: AppRenderContext,
requestStore: RequestStore,
options?: {
actionResult: ActionResult
skipFlight: boolean
componentTree?: CacheNodeSeedData
preloadCallbacks?: PreloadCallbacks
}
requestStore: RequestStore
): Promise<RenderResult> {
const renderOpts = ctx.renderOpts
if (!renderOpts.dev) {
Expand Down Expand Up @@ -622,8 +624,7 @@ async function warmupDevRender(
const rscPayload = await workUnitAsyncStorage.run(
requestStore,
generateDynamicRSCPayload,
ctx,
options
ctx
)

// For app dir, use the bundled version of Flight server renderer (renderToReadableStream)
Expand Down Expand Up @@ -989,12 +990,12 @@ async function renderToHTMLOrFlightImpl(
pagePath: string,
query: NextParsedUrlQuery,
renderOpts: RenderOpts,
requestStore: RequestStore,
workStore: WorkStore,
parsedRequestHeaders: ParsedRequestHeaders,
requestEndedState: { ended?: boolean },
postponedState: PostponedState | null,
implicitTags: Array<string>
implicitTags: Array<string>,
serverComponentsHmrCache: ServerComponentsHmrCache | undefined
) {
const isNotFoundPath = pagePath === '/404'
if (isNotFoundPath) {
Expand Down Expand Up @@ -1047,26 +1048,6 @@ async function renderToHTMLOrFlightImpl(
isNodeNextRequest(req)
) {
req.originalRequest.on('end', () => {
const prerenderStore = workUnitAsyncStorage.getStore()
const isPPR =
prerenderStore &&
(prerenderStore.type === 'prerender' ||
prerenderStore.type === 'prerender-ppr')
? !!prerenderStore.dynamicTracking?.dynamicAccesses?.length
: false

if (
process.env.NODE_ENV === 'development' &&
renderOpts.setAppIsrStatus &&
!isPPR &&
!requestStore.usedDynamic &&
!workStore.forceDynamic
) {
// only node can be ISR so we only need to update the status here
const { pathname } = new URL(req.url || '/', 'http://n')
renderOpts.setAppIsrStatus(pathname, true)
}

requestEndedState.ended = true

if ('performance' in globalThis) {
Expand Down Expand Up @@ -1130,6 +1111,7 @@ async function renderToHTMLOrFlightImpl(
isPrefetchRequest,
isRSCRequest,
isDevWarmupRequest,
isHmrRefresh,
nonce,
} = parsedRequestHeaders

Expand Down Expand Up @@ -1287,10 +1269,44 @@ async function renderToHTMLOrFlightImpl(
return new RenderResult(await streamToString(response.stream), options)
} else {
// We're rendering dynamically
const renderResumeDataCache =
renderOpts.devWarmupRenderResumeDataCache ??
postponedState?.renderResumeDataCache

const requestStore = createRequestStoreForRender(
req,
res,
url,
implicitTags,
renderOpts.onUpdateCookies,
renderOpts.previewProps,
isHmrRefresh,
serverComponentsHmrCache,
renderResumeDataCache
)

if (
process.env.NODE_ENV === 'development' &&
renderOpts.setAppIsrStatus &&
// The type check here ensures that `req` is correctly typed, and the
// environment variable check provides dead code elimination.
process.env.NEXT_RUNTIME !== 'edge' &&
isNodeNextRequest(req)
) {
const setAppIsrStatus = renderOpts.setAppIsrStatus
req.originalRequest.on('end', () => {
if (!requestStore.usedDynamic && !workStore.forceDynamic) {
Copy link
Member

Choose a reason for hiding this comment

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

lgtm, but this'll be clearer if we move it after we instantiate the request store

// only node can be ISR so we only need to update the status here
const { pathname } = new URL(req.url || '/', 'http://n')
setAppIsrStatus(pathname, true)
}
})
}

if (isDevWarmupRequest) {
return warmupDevRender(req, ctx, requestStore)
} else if (isRSCRequest) {
return generateDynamicFlightRenderResult(req, ctx)
return generateDynamicFlightRenderResult(req, ctx, requestStore)
}

const renderToStreamWithTracing = getTracer().wrap(
Expand Down Expand Up @@ -1415,7 +1431,7 @@ export const renderToHTMLOrFlight: AppPageRender = (
isRoutePPREnabled: renderOpts.experimental.isRoutePPREnabled === true,
})

const { isHmrRefresh, isPrefetchRequest } = parsedRequestHeaders
const { isPrefetchRequest } = parsedRequestHeaders

const requestEndedState = { ended: false }
let postponedState: PostponedState | null = null
Expand Down Expand Up @@ -1444,32 +1460,12 @@ export const renderToHTMLOrFlight: AppPageRender = (
)
}

const renderResumeDataCache =
renderOpts.devWarmupRenderResumeDataCache ??
postponedState?.renderResumeDataCache

const implicitTags = getImplicitTags(
renderOpts.routeModule.definition.page,
url,
fallbackRouteParams
)

// TODO: We need to refactor this so that prerenders do not rely upon the
// existence of an outer scoped request store. Then we should move this
// store generation inside the appropriate scope like `renderToStream` where
// we know we're handling a Request and not a Prerender
const requestStore = createRequestStoreForRender(
req,
res,
url,
implicitTags,
renderOpts.onUpdateCookies,
renderResumeDataCache,
renderOpts.previewProps,
isHmrRefresh,
serverComponentsHmrCache
)

const workStore = createWorkStore({
page: renderOpts.routeModule.definition.page,
fallbackRouteParams,
Expand All @@ -1479,24 +1475,24 @@ export const renderToHTMLOrFlight: AppPageRender = (
isPrefetchRequest,
})

return workAsyncStorage.run(workStore, () => {
return workUnitAsyncStorage.run(requestStore, () => {
return renderToHTMLOrFlightImpl(
req,
res,
url,
pagePath,
query,
renderOpts,
requestStore,
workStore,
parsedRequestHeaders,
requestEndedState,
postponedState,
implicitTags
)
})
})
return workAsyncStorage.run(
workStore,
// The function to run
renderToHTMLOrFlightImpl,
// all of it's args
req,
res,
url,
pagePath,
query,
renderOpts,
workStore,
parsedRequestHeaders,
requestEndedState,
postponedState,
implicitTags,
serverComponentsHmrCache
)
}

async function renderToStream(
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/server/async-storage/request-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ export function createRequestStoreForRender(
url: RequestContext['url'],
implicitTags: RequestContext['implicitTags'],
onUpdateCookies: RenderOpts['onUpdateCookies'],
renderResumeDataCache: RenderResumeDataCache | undefined,
previewProps: WrapperRenderOpts['previewProps'],
isHmrRefresh: RequestContext['isHmrRefresh'],
serverComponentsHmrCache: RequestContext['serverComponentsHmrCache']
serverComponentsHmrCache: RequestContext['serverComponentsHmrCache'],
renderResumeDataCache: RenderResumeDataCache | undefined
): RequestStore {
return createRequestStoreImpl(
// Pages start in render phase by default
Expand Down
Loading