Skip to content

Avoid proxying React modules through workUnitStore (#85486) #8

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

Open
MitchLewis930 wants to merge 1 commit into
base: pr_038_before
Choose a base branch
from
Open

Avoid proxying React modules through workUnitStore (#85486) #8

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
18 changes: 1 addition & 17 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 @@ -955,7 +955,6 @@ async function prospectiveRuntimeServerPrerender(
renderResumeDataCache,
prerenderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined,
// We only need task sequencing in the final prerender.
runtimeStagePromise: null,
// These are not present in regular prerenders, but allowed in a runtime prerender.
Expand Down Expand Up @@ -1092,7 +1091,6 @@ async function finalRuntimeServerPrerender(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined,
// Used to separate the "Static" stage from the "Runtime" stage.
runtimeStagePromise,
// These are not present in regular prerenders, but allowed in a runtime prerender.
Expand Down Expand Up @@ -3315,9 +3313,7 @@ async function spawnDynamicValidationInDev(
// ready to cut the render off.
const cacheSignal = new CacheSignal()

const captureOwnerStackClient = ReactClient.captureOwnerStack
const { captureOwnerStack: captureOwnerStackServer, createElement } =
ComponentMod
const { createElement } = ComponentMod

// The resume data cache here should use a fresh instance as it's
// performing a fresh prerender. If we get to implementing the
Expand Down Expand Up @@ -3350,7 +3346,6 @@ async function spawnDynamicValidationInDev(
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash,
captureOwnerStack: captureOwnerStackServer,
}

// We're not going to use the result of this render because the only time it could be used
Expand Down Expand Up @@ -3384,7 +3379,6 @@ async function spawnDynamicValidationInDev(
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash,
captureOwnerStack: captureOwnerStackServer,
}

const pendingInitialServerResult = workUnitAsyncStorage.run(
Expand Down Expand Up @@ -3505,7 +3499,6 @@ async function spawnDynamicValidationInDev(
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash: undefined,
captureOwnerStack: captureOwnerStackClient,
}

const prerender = (
Expand Down Expand Up @@ -3616,7 +3609,6 @@ async function spawnDynamicValidationInDev(
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash,
captureOwnerStack: captureOwnerStackServer,
}

const finalAttemptRSCPayload = await workUnitAsyncStorage.run(
Expand Down Expand Up @@ -3650,7 +3642,6 @@ async function spawnDynamicValidationInDev(
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash,
captureOwnerStack: captureOwnerStackServer,
}

const reactServerResult = await createReactServerPrerenderResult(
Expand Down Expand Up @@ -3730,7 +3721,6 @@ async function spawnDynamicValidationInDev(
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash,
captureOwnerStack: captureOwnerStackClient,
}

let dynamicValidation = createDynamicValidationState()
Expand Down Expand Up @@ -4108,7 +4098,6 @@ async function prerenderToStream(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined, // Not available in production.
}

// We're not going to use the result of this render because the only time it could be used
Expand Down Expand Up @@ -4142,7 +4131,6 @@ async function prerenderToStream(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined, // Not available in production.
})

const pendingInitialServerResult = workUnitAsyncStorage.run(
Expand Down Expand Up @@ -4257,7 +4245,6 @@ async function prerenderToStream(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined, // Not available in production.
}

const prerender = (
Expand Down Expand Up @@ -4367,7 +4354,6 @@ async function prerenderToStream(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined, // Not available in production.
}

const finalAttemptRSCPayload = await workUnitAsyncStorage.run(
Expand Down Expand Up @@ -4402,7 +4388,6 @@ async function prerenderToStream(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined, // Not available in production.
})

let prerenderIsPending = true
Expand Down Expand Up @@ -4488,7 +4473,6 @@ async function prerenderToStream(
prerenderResumeDataCache,
renderResumeDataCache,
hmrRefreshHash: undefined,
captureOwnerStack: undefined, // Not available in production.
}

let dynamicValidation = createDynamicValidationState()
Expand Down
5 changes: 0 additions & 5 deletions packages/next/src/server/app-render/work-unit-async-storage.external.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,6 @@ interface PrerenderStoreModernCommon
* subsequent dynamic render.
*/
readonly hmrRefreshHash: string | undefined

/**
* Only available in dev mode.
*/
readonly captureOwnerStack: undefined | (() => string | null)
}

interface StaticPrerenderStoreCommon {
Expand Down
22 changes: 16 additions & 6 deletions packages/next/src/server/node-environment-extensions/console-dim.external.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,13 +321,23 @@ describe('console-exit patches', () => {
}

// Install patches - this wraps the current console.log
const {
registerGetCacheSignal,
} = require('next/dist/server/node-environment-extensions/console-dim.external')
require('next/dist/server/node-environment-extensions/console-dim.external')

registerGetCacheSignal(() => null)
registerGetCacheSignal(() => controller?.signal)
registerGetCacheSignal(() => null)
const {
registerServerReact,
registerClientReact,
} = require('next/dist/server/runtime-reacts.external')

registerServerReact({
cacheSignal() {
return controller?.signal
},
})
registerClientReact({
cacheSignal() {
return null
},
})

console.log('before abort')
controller.abort()
Expand Down
58 changes: 26 additions & 32 deletions packages/next/src/server/node-environment-extensions/console-dim.external.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@ import {
type ConsoleStore,
} from '../app-render/console-async-storage.external'
import { workUnitAsyncStorage } from '../app-render/work-unit-async-storage.external'

type GetCacheSignal = () => AbortSignal | null
const cacheSignals: Array<GetCacheSignal> = []
export function registerGetCacheSignal(getSignal: GetCacheSignal): void {
cacheSignals.push(getSignal)
}
import { getServerReact, getClientReact } from '../runtime-reacts.external'

// eslint-disable-next-line @typescript-eslint/no-unused-vars -- we may use later and want parity with the HIDDEN_STYLE value
const DIMMED_STYLE = 'dimmed'
Expand Down Expand Up @@ -191,33 +186,32 @@ function patchConsoleMethod(methodName: InterceptableConsoleMethod): void {
// the server React cacheSignal implementation. Any particular console call will be in one, the other, or neither
// scope and these signals return null if you are out of scope so this can be called from a single global patch
// and still work properly.
for (let i = 0; i < cacheSignals.length; i++) {
const signal = cacheSignals[i]() // try to get a signal from registered functions
if (signal) {
// We are in a React Server render and can consult the React cache signal to determine if logs
// are now dimmable.
if (signal.aborted) {
if (currentAbortedLogsStyle === HIDDEN_STYLE) {
return
}
return applyWithDimming.call(
this,
consoleStore,
originalMethod,
methodName,
args
)
} else if (consoleStore?.dim === true) {
return applyWithDimming.call(
this,
consoleStore,
originalMethod,
methodName,
args
)
} else {
return originalMethod.apply(this, args)
const signal =
getClientReact()?.cacheSignal() ?? getServerReact()?.cacheSignal()
Comment on lines +189 to +190
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

The logic change removes the loop that checked multiple signals, but doesn't handle the case where both getClientReact() and getServerReact() return non-null React instances. Previously, the code would iterate through all registered signals until finding one. Now it will only use the first non-null result. Consider whether both React instances could be active simultaneously and whether the fallback logic is correct.

Suggested change
const signal =
getClientReact()?.cacheSignal() ?? getServerReact()?.cacheSignal()
const clientReact = getClientReact()
const serverReact = getServerReact()
const clientSignal = clientReact?.cacheSignal?.()
const serverSignal = serverReact?.cacheSignal?.()
let signal =
clientSignal !== null && clientSignal !== undefined
? clientSignal
: serverSignal !== null && serverSignal !== undefined
? serverSignal
: null
if (
clientSignal &&
serverSignal &&
clientSignal.aborted !== serverSignal.aborted
) {
// If both client and server signals are present but disagree on `aborted`,
// prefer the aborted one so that logs are dimmed/hidden conservatively.
signal = clientSignal.aborted ? clientSignal : serverSignal
}

Copilot uses AI. Check for mistakes.
if (signal) {
// We are in a React Server render and can consult the React cache signal to determine if logs
// are now dimmable.
if (signal.aborted) {
if (currentAbortedLogsStyle === HIDDEN_STYLE) {
return
}
return applyWithDimming.call(
this,
consoleStore,
originalMethod,
methodName,
args
)
} else if (consoleStore?.dim === true) {
return applyWithDimming.call(
this,
consoleStore,
originalMethod,
methodName,
args
)
} else {
return originalMethod.apply(this, args)
}
}

Expand Down
22 changes: 10 additions & 12 deletions packages/next/src/server/node-environment-extensions/utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { workAsyncStorage } from '../app-render/work-async-storage.external'
import {
workUnitAsyncStorage,
type PrerenderStoreModern,
} from '../app-render/work-unit-async-storage.external'
import { workUnitAsyncStorage } from '../app-render/work-unit-async-storage.external'
import {
abortOnSynchronousPlatformIOAccess,
trackSynchronousPlatformIOAccessInDev,
} from '../app-render/dynamic-rendering'
import { InvariantError } from '../../shared/lib/invariant-error'

import { getServerReact, getClientReact } from '../runtime-reacts.external'

type ApiType = 'time' | 'random' | 'crypto'

export function io(expression: string, type: ApiType) {
Expand Down Expand Up @@ -47,7 +46,7 @@ export function io(expression: string, type: ApiType) {
abortOnSynchronousPlatformIOAccess(
workStore.route,
expression,
applyOwnerStack(new Error(message), workUnitStore),
applyOwnerStack(new Error(message)),
workUnitStore
)
}
Expand Down Expand Up @@ -79,7 +78,7 @@ export function io(expression: string, type: ApiType) {
abortOnSynchronousPlatformIOAccess(
workStore.route,
expression,
applyOwnerStack(new Error(message), workUnitStore),
applyOwnerStack(new Error(message)),
workUnitStore
)
}
Expand All @@ -101,16 +100,15 @@ export function io(expression: string, type: ApiType) {
}
}

function applyOwnerStack(error: Error, workUnitStore: PrerenderStoreModern) {
function applyOwnerStack(error: Error) {
// TODO: Instead of stitching the stacks here, we should log the original
// error as-is when it occurs, and let `patchErrorInspect` handle adding the
// owner stack, instead of logging it deferred in the `LogSafely` component
// via `throwIfDisallowedDynamic`.
if (
process.env.NODE_ENV !== 'production' &&
workUnitStore.captureOwnerStack
) {
const ownerStack = workUnitStore.captureOwnerStack()
if (process.env.NODE_ENV !== 'production') {
const ownerStack =
getClientReact()?.captureOwnerStack() ??
getServerReact()?.captureOwnerStack()

if (ownerStack) {
let stack = ownerStack
Expand Down
13 changes: 6 additions & 7 deletions packages/next/src/server/route-modules/app-page/module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ if (process.env.NEXT_RUNTIME !== 'edge') {
vendoredReactSSR =
require('./vendored/ssr/entrypoints') as typeof import('./vendored/ssr/entrypoints')

// In Node environments we augment console logging with information contextual to a React render.
// This patching is global so we need to register the cacheSignal getter from our bundled React instances
// here when we load them rather than in the external module itself when the patch is applied.
const { registerGetCacheSignal } =
require('../../node-environment-extensions/console-dim.external') as typeof import('../../node-environment-extensions/console-dim.external')
registerGetCacheSignal(vendoredReactRSC.React.cacheSignal)
registerGetCacheSignal(vendoredReactSSR.React.cacheSignal)
// In Node environments we need to access the correct React instance from external modules such
// as global patches. We register the loaded React instances here.
const { registerServerReact, registerClientReact } =
require('../../runtime-reacts.external') as typeof import('../../runtime-reacts.external')
registerServerReact(vendoredReactRSC.React)
registerClientReact(vendoredReactSSR.React)
}

/**
Expand Down
2 changes: 0 additions & 2 deletions packages/next/src/server/route-modules/app-route/module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ export class AppRouteRouteModule extends RouteModule<
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash: undefined,
captureOwnerStack: undefined,
})

let prospectiveResult
Expand Down Expand Up @@ -505,7 +504,6 @@ export class AppRouteRouteModule extends RouteModule<
prerenderResumeDataCache,
renderResumeDataCache: null,
hmrRefreshHash: undefined,
captureOwnerStack: undefined,
})

let responseHandled = false
Expand Down
15 changes: 15 additions & 0 deletions packages/next/src/server/runtime-reacts.external.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
let ClientReact: typeof import('react') | null = null
export function registerClientReact(react: typeof import('react')) {
ClientReact = react
}
export function getClientReact() {
return ClientReact
}

let ServerReact: typeof import('react') | null = null
export function registerServerReact(react: typeof import('react')) {
ServerReact = react
}
export function getServerReact() {
return ServerReact
}
1 change: 1 addition & 0 deletions test/production/next-server-nft/next-server-nft.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ const isReact18 = parseInt(process.env.NEXT_TEST_REACT_VERSION) === 18
"/node_modules/next/dist/server/route-modules/pages/vendored/contexts/loadable.js",
"/node_modules/next/dist/server/route-modules/pages/vendored/contexts/router-context.js",
"/node_modules/next/dist/server/route-modules/pages/vendored/contexts/server-inserted-html.js",
"/node_modules/next/dist/server/runtime-reacts.external.js",
"/node_modules/next/dist/shared/lib/deep-freeze.js",
"/node_modules/next/dist/shared/lib/invariant-error.js",
"/node_modules/next/dist/shared/lib/is-plain-object.js",
Expand Down