Re-implement Server Action reducer (#86367)#9
Re-implement Server Action reducer (#86367)#9MitchLewis930 wants to merge 1 commit intopr_039_beforefrom
Conversation
Rewrite of the Server Action reducer to use the PPR/Segment Cache navigation implementation, rather than the old lazy fetch implementation. Server Actions may trigger a revalidation, a redirect, or both. They may also invalidate the cache. The behavior could be naively implemented using router.refresh() and router.push(). Semantically, the routing behavior is equivalent. The main difference is that the server that invokes the action may also send back new data for the page within the same response. Compared to a separate request, this data is more likely to be consistent with any data that may have been mutated by the action, due to global data propagation races. (It's also faster since it avoids an extra server waterfall.) So, navigations initiated by a Server action must be able to "seed" the navigation with the data it just received from the server. I've added a new internal method, navigateToSeededRoute, that implements this behavior.
📝 WalkthroughWalkthroughThis pull request implements seeded navigation with dynamic refresh capabilities, adding seedData and seedHead parameters throughout PPR navigation routines, refactoring refresh operations to use navigateToSeededRoute as a seeded navigation variant, and extending RSC payloads with rendered search queries and interception detection metadata. Changes
Sequence DiagramsequenceDiagram
participant Client as Client Action/Refresh
participant NavHelper as Navigation Helper
participant StartPPR as startPPRNavigation
participant UpdateCache as updateCacheNodeOnNavigation
participant CreateCache as createCacheNodeOnNavigation
participant Server as Server RSC
Client->>NavHelper: navigate/refresh with seedData, seedHead
NavHelper->>StartPPR: call with seedData, seedHead, shouldRefreshDynamicData
alt Has seed data
StartPPR->>UpdateCache: updateCacheNodeOnNavigation(seedData, seedHead, ...)
else No seed data
StartPPR->>CreateCache: createCacheNodeOnNavigation(seedData=null, seedHead=null, ...)
end
UpdateCache->>UpdateCache: Thread seedData/seedHead through child paths
UpdateCache->>CreateCache: Recursive call with seedDataChild, seedHeadChild
CreateCache->>CreateCache: Check if needsDynamicRequest based on seedData presence
alt seedData available and valid
CreateCache->>CreateCache: Use seed data instead of fresh request
else No seed data
CreateCache->>Server: Fetch fresh RSC with dynamic data
Server-->>CreateCache: RSC payload with q, i fields
end
CreateCache-->>UpdateCache: Cache node created
UpdateCache-->>StartPPR: Navigation completed
StartPPR-->>NavHelper: Result
NavHelper-->>Client: Navigation result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/next/src/server/app-render/app-render.tsx (1)
535-558:⚠️ Potential issue | 🟡 MinorHandle array-valued
Varyheaders when computingcouldBeIntercepted.
res.getHeader('vary')can bestring[], which would currently yield a false negative. Consider normalizing to cover both shapes (and keep the logic consistent with the initial payload branch in this file).🔧 Suggested defensive handling
- const varyHeader = ctx.res.getHeader('vary') - const couldBeIntercepted = - typeof varyHeader === 'string' && varyHeader.includes(NEXT_URL) + const varyHeader = ctx.res.getHeader('vary') + const couldBeIntercepted = + typeof varyHeader === 'string' + ? varyHeader.includes(NEXT_URL) + : Array.isArray(varyHeader) + ? varyHeader.some((value) => value.includes(NEXT_URL)) + : false
🤖 Fix all issues with AI agents
In `@packages/next/src/client/components/router-reducer/ppr-navigations.ts`:
- Around line 276-298: The seedData handling incorrectly hardcodes
isSeedRscPartial = false, ignoring CacheNodeSeedData[3]; update the branch that
sets isSeedRscPartial (and the other similar branch around
readCacheNodeFromSeedData) to read and propagate the actual isPartial flag from
seedData (e.g., seedData[3]) into the call to readCacheNodeFromSeedData and into
the computation of needsDynamicRequest so that dynamic requests are triggered
when the seed payload is partial; touch the variables seedRsc, seedLoading,
isSeedRscPartial, isSeedHeadPartial, readCacheNodeFromSeedData, and
needsDynamicRequest to ensure the true partial state is used.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/next/src/client/components/router-reducer/ppr-navigations.tspackages/next/src/client/components/router-reducer/reducers/navigate-reducer.tspackages/next/src/client/components/router-reducer/reducers/refresh-reducer.tspackages/next/src/client/components/router-reducer/reducers/server-action-reducer.tspackages/next/src/client/components/segment-cache/navigation.tspackages/next/src/server/app-render/app-render.tsxpackages/next/src/shared/lib/app-router-types.ts
🧰 Additional context used
🧬 Code graph analysis (4)
packages/next/src/server/app-render/app-render.tsx (1)
packages/next/src/client/route-params.ts (1)
getRenderedSearch(22-38)
packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts (2)
packages/next/src/client/components/router-reducer/reducers/has-interception-route-in-current-tree.ts (1)
hasInterceptionRouteInCurrentTree(4-38)packages/next/src/client/components/segment-cache/navigation.ts (1)
navigateToSeededRoute(209-271)
packages/next/src/client/components/router-reducer/ppr-navigations.ts (1)
packages/next/src/shared/lib/app-router-types.ts (4)
FlightSegmentPath(220-231)CacheNodeSeedData(240-249)HeadData(15-15)ChildSegmentMap(17-17)
packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts (5)
packages/next/src/client/flight-data-helpers.ts (1)
NormalizedFlightData(22-37)packages/next/src/client/components/segment-cache/cache-key.ts (1)
NormalizedSearch(6-6)packages/next/src/client/components/segment-cache/cache.ts (1)
revalidateEntireCache(310-332)packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts (1)
handleExternalUrl(30-42)packages/next/src/client/components/segment-cache/navigation.ts (1)
navigateToSeededRoute(209-271)
🔇 Additional comments (15)
packages/next/src/shared/lib/app-router-types.ts (2)
295-308: NavigationFlightResponse now includes renderedSearch/interception flags — LGTM.
These fields align the shared type with the new payload shape.
311-322: ActionFlightResponse metadata additions look good.
Matches the server/client changes for seeded navigation.packages/next/src/client/components/segment-cache/navigation.ts (6)
69-205:navigatenow threads the refresh flag & same‑page detection — LGTM.
The updated signature and propagation look consistent with the seeded navigation flow.
209-270: Seeded navigation entry point is wired correctly.
The seeded route inputs and dynamic request hookup look consistent.
273-337: Prefetch navigation path aligns with seeded inputs — LGTM.
The null seedData/seedHead + prefetch data split reads correctly.
345-364: Result mapping handles nullable scroll segments cleanly.
This keeps the downstream reducer logic intact.
566-593: No‑prefetch navigation seed resets look correct.
The explicit null seed data/head keeps the simulated prefetch flow consistent.
495-536: No changes needed. TheDynamicRequestTreeForEntireRouteconstant is safe as-is. The object is serialized to a JSON string inprepareFlightRouterStateForRequestbefore any downstream processing, andstripClientOnlyDataFromFlightRouterStatecreates entirely new objects without mutating its input. The original constant is only read, never modified, so mutations cannot leak across navigations. A factory function is unnecessary.Likely an incorrect or invalid review comment.
packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts (1)
160-173: Reducer now passes explicit refresh flag — LGTM.
Keeps the call site aligned with the new navigation API.packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts (1)
23-55: Refresh now goes through seeded navigation with interception-aware nextUrl — nice.
This reads cleanly and matches the new seeded navigation flow.packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts (5)
35-56: Import updates align with the new navigation helpers.
No concerns here.
74-84: Action fetch result now carries renderedSearch/interception metadata — LGTM.
This matches the new payload fields for seeded navigation.
162-233: RSC action response parsing now preserves q/i metadata — looks good.
The conditional capture when flight data exists is appropriate.
291-370: Revalidation/redirect handling and early bailout flow are cleanly structured.
The updated control flow is easier to follow and keeps behavior intact.
387-449: Seeded vs. non‑seeded navigation selection looks solid.
The root-render guard and fallback navigation path are consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| } else if (seedData !== null) { | ||
| // If this navigation was the result of an action, then check if the | ||
| // server sent back data in the action response. We should favor using | ||
| // that, rather than performing a separate request. This is both better | ||
| // for performance and it's more likely to be consistent with any | ||
| // writes that were just performed by the action, compared to a | ||
| // separate request. | ||
| const seedRsc = seedData[0] | ||
| const seedLoading = seedData[2] | ||
| const isSeedRscPartial = false | ||
| const isSeedHeadPartial = seedHead === null | ||
| newCacheNode = readCacheNodeFromSeedData( | ||
| seedRsc, | ||
| seedLoading, | ||
| isSeedRscPartial, | ||
| seedHead, | ||
| isSeedHeadPartial, | ||
| isLeafSegment, | ||
| newParallelRoutes, | ||
| navigatedAt | ||
| ) | ||
| needsDynamicRequest = isLeafSegment && isSeedHeadPartial | ||
| } else if (prefetchData !== null) { |
There was a problem hiding this comment.
Seed data ignores isPartial, risking unresolved dynamic holes.
In both seedData branches, isSeedRscPartial is hardcoded to false. If a seeded payload is partial (CacheNodeSeedData[3] === true), the code will treat it as complete and skip a dynamic request, leaving holes unresolved. Please honor the isPartial flag and propagate it into needsDynamicRequest.
🔧 Proposed fix
- const isSeedRscPartial = false
+ const isSeedRscPartial = seedData[3]
const isSeedHeadPartial = seedHead === null
newCacheNode = readCacheNodeFromSeedData(
seedRsc,
seedLoading,
isSeedRscPartial,
seedHead,
isSeedHeadPartial,
isLeafSegment,
newParallelRoutes,
navigatedAt
)
- needsDynamicRequest = isLeafSegment && isSeedHeadPartial
+ needsDynamicRequest =
+ isSeedRscPartial || (isLeafSegment && isSeedHeadPartial)
@@
- const isSeedRscPartial = false
+ const isSeedRscPartial = seedData[3]
const isSeedHeadPartial = seedHead === null
newCacheNode = readCacheNodeFromSeedData(
seedRsc,
seedLoading,
isSeedRscPartial,
seedHead,
isSeedHeadPartial,
isLeafSegment,
newParallelRoutes,
navigatedAt
)
- needsDynamicRequest = isLeafSegment && isSeedHeadPartial
+ needsDynamicRequest =
+ isSeedRscPartial || (isLeafSegment && isSeedHeadPartial)Also applies to: 584-606
🤖 Prompt for AI Agents
In `@packages/next/src/client/components/router-reducer/ppr-navigations.ts` around
lines 276 - 298, The seedData handling incorrectly hardcodes isSeedRscPartial =
false, ignoring CacheNodeSeedData[3]; update the branch that sets
isSeedRscPartial (and the other similar branch around readCacheNodeFromSeedData)
to read and propagate the actual isPartial flag from seedData (e.g.,
seedData[3]) into the call to readCacheNodeFromSeedData and into the computation
of needsDynamicRequest so that dynamic requests are triggered when the seed
payload is partial; touch the variables seedRsc, seedLoading, isSeedRscPartial,
isSeedHeadPartial, readCacheNodeFromSeedData, and needsDynamicRequest to ensure
the true partial state is used.
2 participants
PR_039
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.