Skip to content

Comments

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

Open
MitchLewis930 wants to merge 1 commit intopr_038_beforefrom
pr_038_after
Open

Avoid proxying React modules through workUnitStore (#85486)#8
MitchLewis930 wants to merge 1 commit intopr_038_beforefrom
pr_038_after

Conversation

@MitchLewis930
Copy link

PR_038

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
@greptile-apps
Copy link

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

This PR refactors how React module functions (like cacheSignal and captureOwnerStack) are accessed from external modules. Instead of proxying specific React functions through workUnitStore, it introduces a centralized registry pattern (runtime-reacts.external.ts) that stores references to both Client and Server React instances.

Key Changes

  • Created new runtime-reacts.external.ts module that provides registerClientReact/getClientReact and registerServerReact/getServerReact functions
  • Removed captureOwnerStack field from PrerenderStoreModernCommon interface in work-unit-async-storage.external.ts
  • Updated console-dim.external.tsx to use the new registry pattern instead of an array-based cacheSignal registry
  • Modified utils.tsx to access captureOwnerStack directly from registered React instances instead of from workUnitStore
  • Updated app-page/module.ts to register full React instances instead of individual functions
  • Removed all captureOwnerStack assignments from prerender store objects throughout app-render.tsx and app-route/module.ts
  • Updated test to use the new registration API

Benefits

  • Cleaner separation of concerns - React functionality no longer needs to be threaded through the store
  • More maintainable - adding new React functions doesn't require store interface changes
  • Consistent pattern for accessing React modules from external code

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The refactoring is well-structured and systematic. It replaces an ad-hoc approach (proxying through workUnitStore) with a cleaner registry pattern. All usages have been updated consistently, tests have been updated, and the NFT output expectations have been adjusted. The changes maintain the same functionality while improving code organization.
  • No files require special attention

Important Files Changed

Filename Overview
packages/next/src/server/runtime-reacts.external.ts New module that provides a registry for accessing React instances (Client and Server) from external modules
packages/next/src/server/app-render/work-unit-async-storage.external.ts Removed captureOwnerStack field from PrerenderStoreModernCommon interface
packages/next/src/server/node-environment-extensions/console-dim.external.tsx Replaced array-based cacheSignal registry with direct React instance access via runtime-reacts module
packages/next/src/server/node-environment-extensions/utils.tsx Updated applyOwnerStack to access captureOwnerStack directly from React instances instead of workUnitStore
packages/next/src/server/route-modules/app-page/module.ts Updated to register full React instances instead of just cacheSignal functions

Sequence Diagram

sequenceDiagram
    participant AppPage as app-page/module.ts
    participant Registry as runtime-reacts.external.ts
    participant ConsoleDim as console-dim.external.tsx
    participant Utils as utils.tsx
    participant React as React Instances

    Note over AppPage: During module initialization
    AppPage->>AppPage: Load vendored React (RSC & SSR)
    AppPage->>Registry: registerServerReact(vendoredReactRSC.React)
    Registry->>Registry: Store ServerReact reference
    AppPage->>Registry: registerClientReact(vendoredReactSSR.React)
    Registry->>Registry: Store ClientReact reference

    Note over ConsoleDim: During console method patch
    ConsoleDim->>Registry: getClientReact()
    Registry-->>ConsoleDim: Return ClientReact instance
    ConsoleDim->>React: ClientReact.cacheSignal()
    React-->>ConsoleDim: AbortSignal | null
    ConsoleDim->>Registry: getServerReact()
    Registry-->>ConsoleDim: Return ServerReact instance
    ConsoleDim->>React: ServerReact.cacheSignal()
    React-->>ConsoleDim: AbortSignal | null

    Note over Utils: During error stack capture
    Utils->>Registry: getClientReact()
    Registry-->>Utils: Return ClientReact instance
    Utils->>React: ClientReact.captureOwnerStack()
    React-->>Utils: ownerStack | null
    Utils->>Registry: getServerReact()
    Registry-->>Utils: Return ServerReact instance
    Utils->>React: ServerReact.captureOwnerStack()
    React-->>Utils: ownerStack | null
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants