Avoid proxying React modules through workUnitStore (#85486)#8
Avoid proxying React modules through workUnitStore (#85486)#8MitchLewis930 wants to merge 1 commit intopr_038_beforefrom
Conversation
Today the `captureOwnerStack()` function is provided to shared utilities through an AsyncLocalStorage that scopes the method from the appropriate React instance. This is so that external code like patches to sync IO methods can still generate errors with the appropriate React owner information even when the patched code itself is not bundled and can be called from etiher SSR or RSC contexts. This works but it makes plumbing the React instances around tricky. There is a simpler way. Most of the time you can just try both React's. If one gives you a non-null/undefined result then you know you are in that scope. If neither do then you're outside a React scope altogether. In this change I remove `captureOwnerStack()` from the workUnitStore types and just call it from the shared server runtime which gives even external code access to the appropriate React instances for bundled code
📝 WalkthroughWalkthroughThis pull request removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/next/src/server/node-environment-extensions/console-dim.external.tsx`:
- Around line 189-215: The code calls cacheSignal() unguarded which will throw
for React builds that don't implement it; update the signal assignment to call
cacheSignal via optional chaining on the method call (e.g.,
getClientReact()?.cacheSignal?.() ?? getServerReact()?.cacheSignal?.()) so that
when getClientReact() or getServerReact() lack cacheSignal the code falls back
to the existing workUnitStore logic; ensure this change preserves the subsequent
use of signal and does not alter the applyWithDimming/originalMethod branches
(symbols: getClientReact, getServerReact, cacheSignal, signal, applyWithDimming,
originalMethod, methodName, args, consoleStore).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/next/src/server/app-render/app-render.tsxpackages/next/src/server/app-render/work-unit-async-storage.external.tspackages/next/src/server/node-environment-extensions/console-dim.external.test.tspackages/next/src/server/node-environment-extensions/console-dim.external.tsxpackages/next/src/server/node-environment-extensions/utils.tsxpackages/next/src/server/route-modules/app-page/module.tspackages/next/src/server/route-modules/app-route/module.tspackages/next/src/server/runtime-reacts.external.tstest/production/next-server-nft/next-server-nft.test.ts
💤 Files with no reviewable changes (2)
- packages/next/src/server/app-render/work-unit-async-storage.external.ts
- packages/next/src/server/route-modules/app-route/module.ts
🧰 Additional context used
🧬 Code graph analysis (4)
packages/next/src/server/app-render/app-render.tsx (1)
packages/next/src/build/templates/edge-ssr-app.ts (1)
ComponentMod(53-53)
packages/next/src/server/node-environment-extensions/utils.tsx (1)
packages/next/src/server/runtime-reacts.external.ts (2)
getClientReact(5-7)getServerReact(13-15)
packages/next/src/server/node-environment-extensions/console-dim.external.test.ts (1)
packages/next/src/server/runtime-reacts.external.ts (2)
registerServerReact(10-12)registerClientReact(2-4)
packages/next/src/server/node-environment-extensions/console-dim.external.tsx (1)
packages/next/src/server/runtime-reacts.external.ts (2)
getClientReact(5-7)getServerReact(13-15)
🔇 Additional comments (8)
test/production/next-server-nft/next-server-nft.test.ts (1)
324-324: Snapshot update for runtime-reacts looks correct.packages/next/src/server/runtime-reacts.external.ts (1)
1-15: LGTM — minimal and clear React instance registry.packages/next/src/server/route-modules/app-page/module.ts (1)
42-47: LGTM — Node runtime now registers the vendored React instances.packages/next/src/server/app-render/app-render.tsx (1)
3316-3316: LGTM — local createElement binding is a clean refactor.packages/next/src/server/node-environment-extensions/console-dim.external.test.ts (1)
324-340: LGTM — test now wires cacheSignal via runtime-reacts registry.packages/next/src/server/node-environment-extensions/utils.tsx (2)
46-52: LGTM — updated applyOwnerStack callsites match new signature.Also applies to: 78-83
103-134: LGTM — owner stack capture now uses runtime-reacts fallback.packages/next/src/server/node-environment-extensions/console-dim.external.tsx (1)
7-7: LGTM — console-dim now sources React instances from runtime-reacts.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const signal = | ||
| getClientReact()?.cacheSignal() ?? getServerReact()?.cacheSignal() | ||
| 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) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React cacheSignal API version introduced when stable
💡 Result:
cacheSignal was introduced as a stable React API in React 19.2.0, released October 1, 2025. [1], [2]
Stable version: react@19.2.0 (and later). [2], [3]
Docs: It’s documented in the React API reference under cacheSignal. [3]
Sources:
[1] React 19.2 release post (Oct 1, 2025) (react.dev)
[2] React changelog / release notes for 19.2.0 (collected.press)
[3] React API reference page for cacheSignal (react.dev)
Citations:
- 1: https://react.dev/blog/2025/10/01/react-19-2?utm_source=openai
- 2: https://collected.press/github/facebook/react/CHANGELOG.md?utm_source=openai
- 3: https://react.dev/reference/react/cacheSignal?utm_source=openai
🏁 Script executed:
# First, let's examine the actual code file to understand the context
cat -n packages/next/src/server/node-environment-extensions/console-dim.external.tsx | head -250 | tail -100Repository: Signal65/next.js-CodeRabbit
Length of output: 4802
🏁 Script executed:
# Search for getClientReact and getServerReact definitions
rg -n "getClientReact|getServerReact" packages/next/src/server/node-environment-extensions/console-dim.external.tsx -A 5Repository: Signal65/next.js-CodeRabbit
Length of output: 691
🏁 Script executed:
# Check if there are any version checks or other guards in the file
rg -n "React\." packages/next/src/server/node-environment-extensions/console-dim.external.tsx | head -20Repository: Signal65/next.js-CodeRabbit
Length of output: 179
Guard cacheSignal method call for React builds that don't implement it.
cacheSignal is only available as a stable API in React 19.2.0 (October 2025). Since Next.js must support older React versions (18.x and earlier 19.x releases), the code will throw when getClientReact() or getServerReact() return objects without the cacheSignal method. This prevents the fallback to workUnitStore from ever executing.
Use optional chaining on the method call itself:
🛠️ Required fix
- const signal =
- getClientReact()?.cacheSignal() ?? getServerReact()?.cacheSignal()
+ const signal =
+ getClientReact()?.cacheSignal?.() ?? getServerReact()?.cacheSignal?.()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const signal = | |
| getClientReact()?.cacheSignal() ?? getServerReact()?.cacheSignal() | |
| 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?.() | |
| 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) | |
| } |
🤖 Prompt for AI Agents
In
`@packages/next/src/server/node-environment-extensions/console-dim.external.tsx`
around lines 189 - 215, The code calls cacheSignal() unguarded which will throw
for React builds that don't implement it; update the signal assignment to call
cacheSignal via optional chaining on the method call (e.g.,
getClientReact()?.cacheSignal?.() ?? getServerReact()?.cacheSignal?.()) so that
when getClientReact() or getServerReact() lack cacheSignal the code falls back
to the existing workUnitStore logic; ensure this change preserves the subsequent
use of signal and does not alter the applyWithDimming/originalMethod branches
(symbols: getClientReact, getServerReact, cacheSignal, signal, applyWithDimming,
originalMethod, methodName, args, consoleStore).
2 participants
PR_038
Summary by CodeRabbit
Release Notes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.