Skip to content

Commit b9e4866

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 b9e4866

File tree

16 files changed

+380
-310
lines changed

16 files changed

+380
-310
lines changed

packages/next/src/client/app-dir/link.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { useMergedRef } from '../use-merged-ref'
88
import { isAbsoluteUrl } from '../../shared/lib/utils'
99
import { addBasePath } from '../add-base-path'
1010
import { warnOnce } from '../../shared/lib/utils/warn-once'
11+
import { ScrollBehavior } from '../components/router-reducer/router-reducer-types'
1112
import type { PENDING_LINK_STATUS } from '../components/links'
1213
import {
1314
IDLE_LINK_STATUS,
@@ -307,7 +308,7 @@ function linkClicked(
307308
dispatchNavigateAction(
308309
href,
309310
replace ? 'replace' : 'push',
310-
scroll ?? true,
311+
scroll === false ? ScrollBehavior.NoScroll : ScrollBehavior.Default,
311312
linkInstanceRef.current,
312313
transitionTypes
313314
)

packages/next/src/client/components/app-router-instance.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
type NavigateAction,
1010
ACTION_HMR_REFRESH,
1111
PrefetchKind,
12+
ScrollBehavior,
1213
type AppHistoryState,
1314
} from './router-reducer/router-reducer-types'
1415
import { reducer } from './router-reducer/router-reducer'
@@ -277,7 +278,7 @@ function getProfilingHookForOnNavigationStart() {
277278
export function dispatchNavigateAction(
278279
href: string,
279280
navigateType: NavigateAction['navigateType'],
280-
shouldScroll: boolean,
281+
scrollBehavior: ScrollBehavior,
281282
linkInstanceRef: LinkInstance | null,
282283
transitionTypes: string[] | undefined
283284
): void {
@@ -307,7 +308,7 @@ export function dispatchNavigateAction(
307308
url,
308309
isExternalUrl: isExternalURL(url),
309310
locationSearch: location.search,
310-
shouldScroll,
311+
scrollBehavior,
311312
navigateType,
312313
})
313314
}
@@ -356,7 +357,10 @@ function gesturePush(href: string, options?: NavigateOptions): void {
356357

357358
// Fork the router state for the duration of the gesture transition.
358359
const currentUrl = new URL(state.canonicalUrl, location.href)
359-
const shouldScroll = options?.scroll ?? true
360+
const scrollBehavior =
361+
options?.scroll === false
362+
? ScrollBehavior.NoScroll
363+
: ScrollBehavior.Default
360364
// This is a special freshness policy that prevents dynamic requests from
361365
// being spawned. During the gesture, we should only show the cached
362366
// prefetched UI, not dynamic data.
@@ -373,7 +377,7 @@ function gesturePush(href: string, options?: NavigateOptions): void {
373377
state.tree,
374378
state.nextUrl,
375379
freshnessPolicy,
376-
shouldScroll,
380+
scrollBehavior,
377381
'push'
378382
)
379383
dispatchGestureState(forkedGestureState)
@@ -442,7 +446,9 @@ export const publicAppRouterInstance: AppRouterInstance = {
442446
dispatchNavigateAction(
443447
href,
444448
'replace',
445-
options?.scroll ?? true,
449+
options?.scroll === false
450+
? ScrollBehavior.NoScroll
451+
: ScrollBehavior.Default,
446452
null,
447453
options?.transitionTypes
448454
)
@@ -458,7 +464,9 @@ export const publicAppRouterInstance: AppRouterInstance = {
458464
dispatchNavigateAction(
459465
href,
460466
'push',
461-
options?.scroll ?? true,
467+
options?.scroll === false
468+
? ScrollBehavior.NoScroll
469+
: ScrollBehavior.Default,
462470
null,
463471
options?.transitionTypes
464472
)

0 commit comments

Comments
 (0)