Skip to content

Comments

Refactor: scope requestStore to dynamic renders only (#72312)#5

Open
MitchLewis930 wants to merge 1 commit intopr_035_beforefrom
pr_035_after
Open

Refactor: scope requestStore to dynamic renders only (#72312)#5
MitchLewis930 wants to merge 1 commit intopr_035_beforefrom
pr_035_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_035

Summary by CodeRabbit

  • Refactor
    • Strengthened request-scoped state handling for server actions and dynamic rendering to ensure consistent execution context.
    • Action execution and flight generation now run within a request-scoped context, improving reliability of server-side updates and error paths.
    • Adjusted render pipeline and request-store plumbing to better propagate HMR and render-resume data across render flows.

This completes the refactor to eliminate a requestStore scoped around
prerenders. Now we only scope requestStore around dynamic renders. If
you are prerendering then the workUnitAsyncStorage will only have a
prerneder store or undefined. While it is possible to shadow stores
because you can enter a cache scope from a render or prerender it
generally should never be the case that you enter a prerender from a
request or enter a request from a prerender. These are effectively top
level scopes.

This should not change any program behavior

stacked on vercel#72212
@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Propagates request-scoped context through rendering and server action flows by introducing workUnitAsyncStorage wrapping for action execution, adding requestStore parameters to flight/generation call sites, reordering request-store factory parameters, and updating render function signatures to include serverComponentsHmrCache.

Changes

Cohort / File(s) Summary
Action handler async storage
packages/next/src/server/app-render/action-handler.ts
Adds workUnitAsyncStorage import and runs actionHandler.apply(...) inside workUnitAsyncStorage.run(requestStore, ...). Propagates requestStore to finalizeAndGenerateFlight and generateFlight call sites across success and error branches.
App render pipeline & flight generation
packages/next/src/server/app-render/app-render.tsx
Adds requestStore propagation to dynamic flight rendering (generateDynamicFlightRenderResult, warmupDevRender) and wraps dynamic render calls with workUnitAsyncStorage.run(requestStore, ...). Removes ad-hoc requestStore creation and updates renderToHTMLOrFlightImpl signature to accept serverComponentsHmrCache while removing requestStore from that impl. Updates AppPageRender type to include serverComponentsHmrCache.
Request store factory signature
packages/next/src/server/async-storage/request-store.ts
Reorders parameters in createRequestStoreForRender so serverComponentsHmrCache appears before renderResumeDataCache, updating the public function signature while preserving internal wiring.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ActionHandler as Action Handler
    participant AsyncStorage as workUnitAsyncStorage
    participant RequestStore as RequestStore
    participant FlightGen as Flight Generation

    Client->>ActionHandler: invoke server action
    ActionHandler->>AsyncStorage: run(requestStore, callback)
    AsyncStorage->>RequestStore: establish request-scoped context
    AsyncStorage->>ActionHandler: execute handler within context
    ActionHandler->>FlightGen: finalizeAndGenerateFlight(req, ctx, requestStore, ...)
    FlightGen->>RequestStore: read/write request-scoped data
    FlightGen-->>ActionHandler: return flight payload
    ActionHandler-->>Client: send response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐇✨ I hopped in code where contexts hide,
Wrapped actions in a work-unit tide,
RequestStore held the thread so mild,
Flights now carry context filed—
A rabbit bounds, async and spry.

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning Description is incomplete; only contains 'PR_035' with no actual details about changes, rationale, or context required by the template. Add a proper description following the template: explain what was changed and why. Include context about requestStore scoping to dynamic renders and prerender implications.
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the main refactoring: scoping requestStore to dynamic renders only, matching the core change across all modified files.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into pr_035_before

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pr_035_after

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
packages/next/src/server/app-render/action-handler.ts (1)

1001-1009: Inconsistent cookie synchronization in generic error path.

In this error handling branch (500 error), generateFlight is called directly with the phase manually set at line 1001, but synchronizeMutableCookies(requestStore) is not called. Other error paths (CSRF error at line 565, not-found at line 969) use finalizeAndGenerateFlight, which includes the cookie synchronization step.

This inconsistency predates this PR (only requestStore parameter was added), but it may cause cookies mutated during the action to not be reflected in the error response for generic exceptions, while they would be for CSRF or not-found errors.

♻️ Consider using `finalizeAndGenerateFlight` for consistency
      requestStore.phase = 'render'
      return {
        type: 'done',
-       result: await generateFlight(req, ctx, requestStore, {
+       result: await finalizeAndGenerateFlight(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,
          temporaryReferences,
        }),
      }
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 719a187 and 071a059.

📒 Files selected for processing (3)
  • packages/next/src/server/app-render/action-handler.ts
  • packages/next/src/server/app-render/app-render.tsx
  • packages/next/src/server/async-storage/request-store.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/next/src/server/app-render/action-handler.ts (6)
packages/next/src/server/lib/mock-request.ts (1)
  • req (388-390)
packages/next/src/server/web/sandbox/context.ts (1)
  • requestStore (243-245)
packages/next/src/server/app-render/work-unit-async-storage.external.ts (1)
  • workUnitAsyncStorage (184-184)
packages/next/src/server/app-render/entry-base.ts (1)
  • workUnitAsyncStorage (60-60)
packages/next/src/server/app-render/work-unit-async-storage-instance.ts (1)
  • workUnitAsyncStorage (4-5)
packages/next/src/build/templates/app-route.ts (1)
  • workUnitAsyncStorage (47-47)
packages/next/src/server/async-storage/request-store.ts (2)
packages/next/src/server/base-server.ts (1)
  • RequestContext (298-307)
packages/next/src/server/resume-data-cache/resume-data-cache.ts (1)
  • RenderResumeDataCache (14-26)
packages/next/src/server/app-render/app-render.tsx (7)
packages/next/src/server/web/sandbox/context.ts (1)
  • requestStore (243-245)
packages/next/src/server/app-render/work-unit-async-storage.external.ts (2)
  • RequestStore (24-69)
  • workUnitAsyncStorage (184-184)
packages/next/src/server/app-render/entry-base.ts (2)
  • workUnitAsyncStorage (60-60)
  • workAsyncStorage (59-59)
packages/next/src/server/app-render/work-unit-async-storage-instance.ts (1)
  • workUnitAsyncStorage (4-5)
packages/next/src/server/async-storage/request-store.ts (1)
  • createRequestStoreForRender (105-129)
packages/next/src/shared/lib/router/router.ts (2)
  • pathname (2567-2569)
  • query (2571-2573)
packages/next/src/server/app-render/work-async-storage.external.ts (1)
  • workAsyncStorage (76-76)
🔇 Additional comments (9)
packages/next/src/server/async-storage/request-store.ts (1)

105-128: LGTM!

The parameter reordering for createRequestStoreForRender is correctly propagated to the internal createRequestStoreImpl call. The new order places serverComponentsHmrCache before renderResumeDataCache in the public API, which aligns with how callers construct these values (HMR cache is typically available earlier in the call chain).

packages/next/src/server/app-render/action-handler.ts (3)

48-48: LGTM!

The import of workUnitAsyncStorage is required for the new request-scoped context wrapping introduced in this refactor.


882-884: Core change: action handler now executes within request-scoped storage context.

This correctly scopes the action execution within workUnitAsyncStorage.run, ensuring that the requestStore context is available throughout the action handler invocation. This aligns with the PR objective of scoping requestStore to dynamic renders only.


565-571: LGTM!

All three finalizeAndGenerateFlight call sites correctly pass requestStore as the third parameter, matching the updated GenerateFlight type signature. The wrapper properly synchronizes mutable cookies and sets the render phase before generating the flight response.

Also applies to: 893-898, 969-974

packages/next/src/server/app-render/app-render.tsx (5)

509-582: LGTM!

The generateDynamicFlightRenderResult function is correctly updated to:

  1. Accept requestStore as a parameter
  2. Wrap RSCPayload generation in workUnitAsyncStorage.run(requestStore, ...)
  3. Wrap renderToReadableStream in workUnitAsyncStorage.run(requestStore, ...)

This ensures the request-scoped context is available throughout the dynamic flight render process.


591-661: LGTM!

The warmupDevRender function correctly accepts requestStore and uses it to:

  1. Set devWarmupPrerenderResumeDataCache on the request store
  2. Wrap RSC payload and stream generation in workUnitAsyncStorage.run
  3. Clean up by nullifying the prerender cache reference after rendering

The warmup flow properly scopes the request context for development-time cache pre-population.


1272-1309: LGTM!

The dynamic render branch correctly:

  1. Creates requestStore with the updated parameter order (serverComponentsHmrCache before renderResumeDataCache)
  2. Adds dev-mode ISR status tracking that checks requestStore.usedDynamic on request end
  3. Passes requestStore to both warmupDevRender and generateDynamicFlightRenderResult

The ISR status tracking at lines 1288-1304 is a nice addition that provides accurate ISR eligibility feedback in development by monitoring whether dynamic APIs were accessed during the request.


1478-1495: LGTM!

The workAsyncStorage.run call correctly propagates all parameters to renderToHTMLOrFlightImpl, including the new serverComponentsHmrCache as the final parameter. This matches the updated function signature and ensures the HMR cache is available throughout the render pipeline.


186-186: Type alias correctly reflects the updated signature.

The GenerateFlight type alias is derived from generateDynamicFlightRenderResult, which now includes the requestStore parameter. All call sites in action-handler.ts that use this type have been updated accordingly.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@MitchLewis930
Copy link
Author

@CodeRabbit full review

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

✅ Actions performed

Full review triggered.

@MitchLewis930
Copy link
Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

✅ Actions performed

Full review triggered.

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