Skip to content

Commit

Permalink
[dynamicIO] update controller to be non-optional (vercel#72048)
Browse files Browse the repository at this point in the history
Previously we used the controller a signal for whether the current
dynamicIO prerender should abort early or not. Now we separate the
controller which signals whether we've left the Prerneder phase from the
signal that actually aborts the render in some cases. Now we can always
scope a controller in the prerenderStore and depending on whether we are
filling caches or not we will end the render early or let it continue.

With this change the controller is always provided so we can reflect
this in the type
  • Loading branch information
gnoff authored and stipsan committed Nov 6, 2024
1 parent e0fdabf commit 531019c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 17 deletions.
4 changes: 1 addition & 3 deletions packages/next/src/server/app-render/dynamic-rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,7 @@ function abortOnSynchronousDynamicDataAccess(

const error = createPrerenderInterruptedError(reason)

if (prerenderStore.controller) {
prerenderStore.controller.abort(error)
}
prerenderStore.controller.abort(error)

const dynamicTracking = prerenderStore.dynamicTracking
if (dynamicTracking) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,25 @@ export type RequestStore = {
export type PrerenderStoreModern = {
type: 'prerender'
readonly implicitTags: string[]

/**
* This signal is aborted when the React render is complete. (i.e. it is the same signal passed to react)
*/
readonly renderSignal: AbortSignal
/**
* This is the AbortController passed to React. It can be used to abort the prerender
* if we encounter conditions that do not require further rendering
* This is the AbortController which represents the boundary between Prerender and dynamic. In some renders it is
* the same as the controller for the renderSignal but in others it is a separate controller. It should be aborted
* whenever the we are no longer in the prerender phase of rendering. Typically this is after one task or when you call
* a sync API which requires the prerender to end immediately
*/
readonly controller: null | AbortController
readonly controller: AbortController

/**
* when not null this signal is used to track cache reads during prerendering and
* to await all cache reads completing before aborting the prerender.
*/
readonly cacheSignal: null | CacheSignal

/**
* This signal is used to clean up the prerender once it is complete.
*/
readonly renderSignal: AbortSignal

/**
* During some prerenders we want to track dynamic access.
*/
Expand Down
14 changes: 8 additions & 6 deletions packages/next/src/server/route-modules/app-route/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ import {
postponeWithTracking,
createDynamicTrackingState,
getFirstDynamicReason,
isPrerenderInterruptedError,
} from '../../app-render/dynamic-rendering'
import { ReflectAdapter } from '../../web/spec-extension/adapters/reflect'
import type { RenderOptsPartial } from '../../app-render/types'
Expand Down Expand Up @@ -349,10 +348,10 @@ export class AppRouteRouteModule extends RouteModule<
phase: 'action',
implicitTags: implicitTags,
renderSignal: prospectiveController.signal,
controller: prospectiveController,
cacheSignal,
// During prospective render we don't use a controller
// because we need to let all caches fill.
controller: null,
dynamicTracking,
revalidate: defaultRevalidate,
expire: INFINITE_CACHE,
Expand All @@ -369,11 +368,14 @@ export class AppRouteRouteModule extends RouteModule<
handlerContext
)
} catch (err) {
if (isPrerenderInterruptedError(err)) {
if (prospectiveController.signal.aborted) {
// the route handler called an API which is always dynamic
// there is no need to try again
prospectiveRenderIsDynamic = true
} else if (process.env.NEXT_DEBUG_BUILD) {
} else if (
process.env.NEXT_DEBUG_BUILD ||
process.env.__NEXT_VERBOSE_LOGGING
) {
printDebugThrownValueForProspectiveRender(err, workStore.route)
}
}
Expand All @@ -387,7 +389,7 @@ export class AppRouteRouteModule extends RouteModule<
;(prospectiveResult as any as Promise<unknown>).then(
() => {},
(err) => {
if (isPrerenderInterruptedError(err)) {
if (prospectiveController.signal.aborted) {
// the route handler called an API which is always dynamic
// there is no need to try again
prospectiveRenderIsDynamic = true
Expand Down Expand Up @@ -431,8 +433,8 @@ export class AppRouteRouteModule extends RouteModule<
phase: 'action',
implicitTags: implicitTags,
renderSignal: finalController.signal,
cacheSignal: null,
controller: finalController,
cacheSignal: null,
dynamicTracking,
revalidate: defaultRevalidate,
expire: INFINITE_CACHE,
Expand Down

0 comments on commit 531019c

Please sign in to comment.