Skip to content

Commit 2618f59

Browse files
committed
Simplify scroll restoration with shared ScrollRef on CacheNode
The scroll system previously relied on accumulating segment paths during navigation, then matching those paths in layout-router to decide which segments should scroll. This was necessary when CacheNodes were created lazily during render, since we couldn't mark scroll targets on the nodes themselves. But now that we construct the entire CacheNode tree immediately upon navigation, we can use a much simpler approach. Each navigation creates a single shared mutable ref (ScrollRef) and assigns it to every new leaf CacheNode. When any segment's scroll handler fires, it sets the ref to false, preventing other segments from scrolling. This replaces all the segment path accumulation and matching logic with a straightforward per-node flag. The motivation for this refactor was a bug where calling refresh() from a server action would scroll the page to the top. The old path-based system had a semantic gap between null (no segments to scroll) and an empty array (scroll everything) — when a server action triggered a refresh with no new segments, the null value fell through to a code path that scrolled unconditionally. The new model avoids this entirely: refresh creates no new CacheNodes, so no ScrollRef is assigned, and nothing scrolls. There is extensive existing test coverage for scroll restoration behavior. This adds one additional test for the server action refresh bug.
1 parent 1126661 commit 2618f59

File tree

10 files changed

+324
-277
lines changed

10 files changed

+324
-277
lines changed

packages/next/src/client/components/layout-router.tsx

Lines changed: 143 additions & 175 deletions
Large diffs are not rendered by default.

packages/next/src/client/components/router-reducer/create-initial-router-state.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,10 +233,9 @@ export function createInitialRouterState({
233233
preserveCustomHistoryState: true,
234234
},
235235
focusAndScrollRef: {
236-
apply: false,
236+
scrollRef: null,
237237
onlyHashChange: false,
238238
hashFragment: null,
239-
segmentPaths: [],
240239
},
241240
canonicalUrl,
242241
renderedSearch: initialRenderedSearch,

packages/next/src/client/components/router-reducer/ppr-navigations.ts

Lines changed: 50 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import type {
22
CacheNodeSeedData,
33
FlightRouterState,
4-
FlightSegmentPath,
54
Segment,
65
} from '../../../shared/lib/app-router-types'
76
import type { CacheNode } from '../../../shared/lib/app-router-types'
8-
import type { HeadData } from '../../../shared/lib/app-router-types'
7+
import type { HeadData, ScrollRef } from '../../../shared/lib/app-router-types'
98
import { PrefetchHint } from '../../../shared/lib/app-router-types'
109
import {
1110
PAGE_SEGMENT_KEY,
@@ -118,8 +117,14 @@ const enum NavigationTaskExitStatus {
118117
}
119118

120119
export type NavigationRequestAccumulation = {
121-
scrollableSegments: Array<FlightSegmentPath> | null
122120
separateRefreshUrls: Set<string> | null
121+
/**
122+
* A shared mutable ref assigned to all new leaf segments during this
123+
* navigation. If no new segments are created (e.g. during a refresh),
124+
* this stays null — signaling that the previous navigation's scroll
125+
* targets should be preserved.
126+
*/
127+
scrollRef: ScrollRef | null
123128
}
124129

125130
const noop = () => {}
@@ -133,8 +138,8 @@ export function createInitialCacheNodeForHydration(
133138
// Create the initial cache node tree, using the data embedded into the
134139
// HTML document.
135140
const accumulation: NavigationRequestAccumulation = {
136-
scrollableSegments: null,
137141
separateRefreshUrls: null,
142+
scrollRef: null,
138143
}
139144
const task = createCacheNodeOnNavigation(
140145
navigatedAt,
@@ -143,11 +148,14 @@ export function createInitialCacheNodeForHydration(
143148
FreshnessPolicy.Hydration,
144149
seedData,
145150
seedHead,
146-
null,
147-
null,
148151
false,
149152
accumulation
150153
)
154+
// The initial hydration should not trigger a scroll. Neutralize any
155+
// scroll ref that was created during tree construction.
156+
if (accumulation.scrollRef !== null) {
157+
accumulation.scrollRef.current = false
158+
}
151159
return task
152160
}
153161

@@ -213,8 +221,6 @@ export function startPPRNavigation(
213221
seedData,
214222
seedHead,
215223
isSamePageNavigation,
216-
null,
217-
null,
218224
parentNeedsDynamicRequest,
219225
oldRootRefreshState,
220226
parentRefreshState,
@@ -234,8 +240,6 @@ function updateCacheNodeOnNavigation(
234240
seedData: CacheNodeSeedData | null,
235241
seedHead: HeadData | null,
236242
isSamePageNavigation: boolean,
237-
parentSegmentPath: FlightSegmentPath | null,
238-
parentParallelRouteKey: string | null,
239243
parentNeedsDynamicRequest: boolean,
240244
oldRootRefreshState: RefreshState,
241245
parentRefreshState: RefreshState | null,
@@ -285,37 +289,18 @@ function updateCacheNodeOnNavigation(
285289
) {
286290
return null
287291
}
288-
if (parentSegmentPath === null || parentParallelRouteKey === null) {
289-
// The root should never mismatch. If it does, it suggests an internal
290-
// Next.js error, or a malformed server response. Trigger a full-
291-
// page navigation.
292-
return null
293-
}
294292
return createCacheNodeOnNavigation(
295293
navigatedAt,
296294
newRouteTree,
297295
newMetadataVaryPath,
298296
freshness,
299297
seedData,
300298
seedHead,
301-
parentSegmentPath,
302-
parentParallelRouteKey,
303299
parentNeedsDynamicRequest,
304300
accumulation
305301
)
306302
}
307303

308-
// TODO: The segment paths are tracked so that LayoutRouter knows which
309-
// segments to scroll to after a navigation. But we should just mark this
310-
// information on the CacheNode directly. It used to be necessary to do this
311-
// separately because CacheNodes were created lazily during render, not when
312-
// rather than when creating the route tree.
313-
const segmentPath =
314-
parentParallelRouteKey !== null && parentSegmentPath !== null
315-
? parentSegmentPath.concat([parentParallelRouteKey, newSegment])
316-
: // NOTE: The root segment is intentionally omitted from the segment path
317-
[]
318-
319304
const newSlots = newRouteTree.slots
320305
const oldRouterStateChildren = oldRouterState[1]
321306
const seedDataChildren = seedData !== null ? seedData[1] : null
@@ -327,22 +312,31 @@ function updateCacheNodeOnNavigation(
327312
didFindRootLayout ||
328313
(newRouteTree.prefetchHints & PrefetchHint.IsRootLayout) !== 0
329314

330-
let shouldRefreshDynamicData: boolean = false
315+
let shouldRefreshDynamicData: boolean
316+
// Whether reused leaf segments should be marked as scroll targets.
317+
// Only true for genuine navigations; refreshes and history traversals
318+
// should not trigger a scroll.
319+
let shouldScrollReusedLeaf: boolean
331320
switch (freshness) {
332321
case FreshnessPolicy.Default:
322+
case FreshnessPolicy.Gesture:
323+
shouldRefreshDynamicData = false
324+
shouldScrollReusedLeaf = true
325+
break
333326
case FreshnessPolicy.HistoryTraversal:
334327
case FreshnessPolicy.Hydration: // <- shouldn't happen during client nav
335-
case FreshnessPolicy.Gesture:
336-
// We should never drop dynamic data in shared layouts, except during
337-
// a refresh.
338328
shouldRefreshDynamicData = false
329+
shouldScrollReusedLeaf = false
339330
break
340331
case FreshnessPolicy.RefreshAll:
341332
case FreshnessPolicy.HMRRefresh:
342333
shouldRefreshDynamicData = true
334+
shouldScrollReusedLeaf = false
343335
break
344336
default:
345337
freshness satisfies never
338+
shouldRefreshDynamicData = false
339+
shouldScrollReusedLeaf = false
346340
break
347341
}
348342

@@ -369,6 +363,16 @@ function updateCacheNodeOnNavigation(
369363
const dropPrefetchRsc = false
370364
newCacheNode = reuseSharedCacheNode(dropPrefetchRsc, oldCacheNode)
371365
needsDynamicRequest = false
366+
367+
if (isLeafSegment && shouldScrollReusedLeaf) {
368+
// Lazily create a single ScrollRef shared by all leaves in this
369+
// navigation. The first segment to scroll sets current to false,
370+
// preventing the others from also scrolling.
371+
if (accumulation.scrollRef === null) {
372+
accumulation.scrollRef = { current: true }
373+
}
374+
newCacheNode.scrollRef = accumulation.scrollRef
375+
}
372376
} else {
373377
// If this is part of a refresh, ignore the existing CacheNode and create a
374378
// new one.
@@ -501,8 +505,6 @@ function updateCacheNodeOnNavigation(
501505
seedDataChild ?? null,
502506
seedHeadChild,
503507
isSamePageNavigation,
504-
segmentPath,
505-
parallelRouteKey,
506508
parentNeedsDynamicRequest || needsDynamicRequest,
507509
oldRootRefreshState,
508510
refreshState,
@@ -572,8 +574,6 @@ function createCacheNodeOnNavigation(
572574
freshness: FreshnessPolicy,
573575
seedData: CacheNodeSeedData | null,
574576
seedHead: HeadData | null,
575-
parentSegmentPath: FlightSegmentPath | null,
576-
parentParallelRouteKey: string | null,
577577
parentNeedsDynamicRequest: boolean,
578578
accumulation: NavigationRequestAccumulation
579579
): NavigationTask {
@@ -588,33 +588,10 @@ function createCacheNodeOnNavigation(
588588
// diverges, which is why we keep them separate.
589589

590590
const newSegment = createSegmentFromRouteTree(newRouteTree)
591-
const segmentPath =
592-
parentParallelRouteKey !== null && parentSegmentPath !== null
593-
? parentSegmentPath.concat([parentParallelRouteKey, newSegment])
594-
: // NOTE: The root segment is intentionally omitted from the segment path
595-
[]
596591

597592
const newSlots = newRouteTree.slots
598593
const seedDataChildren = seedData !== null ? seedData[1] : null
599594

600-
const isLeafSegment = newSlots === null
601-
602-
if (isLeafSegment) {
603-
// The segment path of every leaf segment (i.e. page) is collected into
604-
// a result array. This is used by the LayoutRouter to scroll to ensure that
605-
// new pages are visible after a navigation.
606-
//
607-
// This only happens for new pages, not for refreshed pages.
608-
//
609-
// TODO: We should use a string to represent the segment path instead of
610-
// an array. We already use a string representation for the path when
611-
// accessing the Segment Cache, so we can use the same one.
612-
if (accumulation.scrollableSegments === null) {
613-
accumulation.scrollableSegments = []
614-
}
615-
accumulation.scrollableSegments.push(segmentPath)
616-
}
617-
618595
const seedRsc = seedData !== null ? seedData[0] : null
619596
const result = createCacheNodeForSegment(
620597
navigatedAt,
@@ -627,6 +604,18 @@ function createCacheNodeOnNavigation(
627604
const newCacheNode = result.cacheNode
628605
const needsDynamicRequest = result.needsDynamicRequest
629606

607+
const isLeafSegment = newSlots === null
608+
if (isLeafSegment) {
609+
// Mark leaf segments (pages) for scrolling after navigation.
610+
// Lazily create a single ScrollRef shared by all leaves in this
611+
// navigation. The first segment to scroll sets current to false,
612+
// preventing the others from also scrolling.
613+
if (accumulation.scrollRef === null) {
614+
accumulation.scrollRef = { current: true }
615+
}
616+
newCacheNode.scrollRef = accumulation.scrollRef
617+
}
618+
630619
let patchedRouterStateChildren: {
631620
[parallelRouteKey: string]: FlightRouterState
632621
} = {}
@@ -653,8 +642,6 @@ function createCacheNodeOnNavigation(
653642
freshness,
654643
seedDataChild ?? null,
655644
seedHead,
656-
segmentPath,
657-
parallelRouteKey,
658645
parentNeedsDynamicRequest || needsDynamicRequest,
659646
accumulation
660647
)
@@ -1199,6 +1186,7 @@ function createCacheNode(
11991186
head,
12001187
prefetchHead,
12011188
slots: null,
1189+
scrollRef: null,
12021190
}
12031191
}
12041192

packages/next/src/client/components/router-reducer/reducers/restore-reducer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ export function restoreReducer(
4545

4646
const now = Date.now()
4747
const accumulation: NavigationRequestAccumulation = {
48-
scrollableSegments: null,
4948
separateRefreshUrls: null,
49+
scrollRef: null,
5050
}
5151
const restoreSeed = convertServerPatchToFullTree(
5252
treeToRestore,

packages/next/src/client/components/router-reducer/router-reducer-types.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
1-
import type { CacheNode } from '../../../shared/lib/app-router-types'
2-
import type {
3-
FlightRouterState,
4-
FlightSegmentPath,
5-
} from '../../../shared/lib/app-router-types'
1+
import type { CacheNode, ScrollRef } from '../../../shared/lib/app-router-types'
2+
import type { FlightRouterState } from '../../../shared/lib/app-router-types'
63
import type { NavigationSeed } from '../segment-cache/navigation'
74
import type { FetchServerResponseResult } from './fetch-server-response'
85

@@ -165,17 +162,24 @@ export interface PushRef {
165162

166163
export type FocusAndScrollRef = {
167164
/**
168-
* If focus and scroll should be set in the layout-router's useEffect()
169-
*/
170-
apply: boolean
165+
* A shared mutable ref that controls whether a scroll should happen.
166+
* When set, the first segment whose scroll handler fires will scroll
167+
* and set `current` to `false`, preventing other segments from also
168+
* scrolling.
169+
*
170+
* The scroll handler checks `cacheNode.scrollRef` first (per-node),
171+
* then falls back to this (top-level). This ensures that even if a
172+
* subsequent reducer (e.g. refresh) rebuilds the cache tree without
173+
* per-node refs, the scroll intent from a prior reducer (e.g. push)
174+
* is preserved.
175+
*
176+
* `null` means no scroll should happen.
177+
*/
178+
scrollRef: ScrollRef | null
171179
/**
172180
* The hash fragment that should be scrolled to.
173181
*/
174182
hashFragment: string | null
175-
/**
176-
* The paths of the segments that should be focused.
177-
*/
178-
segmentPaths: FlightSegmentPath[]
179183
/**
180184
* If only the URLs hash fragment changed
181185
*/

0 commit comments

Comments
 (0)