From 9c42a7dbc5b78e002f3ec4687674735e92ba6473 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Tue, 27 Jun 2023 12:48:52 +0200 Subject: [PATCH 1/9] WIP scroll: false support --- .../next/src/client/components/app-router.tsx | 25 +++++++++++------- .../router-reducer/handle-mutable.ts | 26 ++++++++++++------- .../reducers/navigate-reducer.ts | 5 +++- .../router-reducer/router-reducer-types.ts | 5 +++- .../next/src/shared/lib/app-router-context.ts | 1 + 5 files changed, 41 insertions(+), 21 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 804e648a82155..1b31d2bb9d591 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -223,11 +223,7 @@ function Router({ ) const navigate: RouterNavigate = useCallback( - ( - href: string, - navigateType: 'push' | 'replace', - forceOptimisticNavigation: boolean - ) => { + (href, navigateType, forceOptimisticNavigation, shouldScroll) => { const url = new URL(addBasePath(href), location.href) return dispatch({ @@ -236,6 +232,7 @@ function Router({ isExternalUrl: isExternalURL(url), locationSearch: location.search, forceOptimisticNavigation, + shouldScroll, navigateType, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -292,16 +289,26 @@ function Router({ }) }) }, - replace: (href, options = {}) => { + replace: (href, options = { scroll: true }) => { // @ts-ignore startTransition exists React.startTransition(() => { - navigate(href, 'replace', Boolean(options.forceOptimisticNavigation)) + navigate( + href, + 'replace', + Boolean(options.forceOptimisticNavigation), + options.scroll + ) }) }, - push: (href, options = {}) => { + push: (href, options = { scroll: true }) => { // @ts-ignore startTransition exists React.startTransition(() => { - navigate(href, 'push', Boolean(options.forceOptimisticNavigation)) + navigate( + href, + 'push', + Boolean(options.forceOptimisticNavigation), + options.scroll + ) }) }, refresh: () => { diff --git a/packages/next/src/client/components/router-reducer/handle-mutable.ts b/packages/next/src/client/components/router-reducer/handle-mutable.ts index 82c829f9be76d..4f6d5f8939de3 100644 --- a/packages/next/src/client/components/router-reducer/handle-mutable.ts +++ b/packages/next/src/client/components/router-reducer/handle-mutable.ts @@ -30,19 +30,25 @@ export function handleMutable( }, // All navigation requires scroll and focus management to trigger. focusAndScrollRef: { - apply: - mutable?.scrollableSegments !== undefined + apply: mutable.shouldScroll + ? mutable?.scrollableSegments !== undefined ? true - : state.focusAndScrollRef.apply, - hashFragment: - // Empty hash should trigger default behavior of scrolling layout into view. - // #top is handled in layout-router. - mutable.hashFragment && mutable.hashFragment !== '' + : state.focusAndScrollRef.apply + : // If shouldScroll is false then we should not apply scroll and focus management. + false, + hashFragment: mutable.shouldScroll + ? // Empty hash should trigger default behavior of scrolling layout into view. + // #top is handled in layout-router. + mutable.hashFragment && mutable.hashFragment !== '' ? // Remove leading # and decode hash to make non-latin hashes work. decodeURIComponent(mutable.hashFragment.slice(1)) - : state.focusAndScrollRef.hashFragment, - segmentPaths: - mutable?.scrollableSegments ?? state.focusAndScrollRef.segmentPaths, + : state.focusAndScrollRef.hashFragment + : // If shouldScroll is false then we should not apply scroll and focus management. + null, + segmentPaths: mutable.shouldScroll + ? mutable?.scrollableSegments ?? state.focusAndScrollRef.segmentPaths + : // If shouldScroll is false then we should not apply scroll and focus management. + [], }, // Apply cache. cache: mutable.cache ? mutable.cache : state.cache, diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts index d8c83012dc3df..026701ebae795 100644 --- a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts @@ -114,6 +114,7 @@ export function navigateReducer( cache, mutable, forceOptimisticNavigation, + shouldScroll, } = action const { pathname, hash } = url const href = createHrefFromUrl(url) @@ -191,6 +192,7 @@ export function navigateReducer( mutable.patchedTree = optimisticTree mutable.pendingPush = pendingPush mutable.hashFragment = hash + mutable.shouldScroll = shouldScroll mutable.scrollableSegments = [] mutable.cache = temporaryCacheNode mutable.canonicalUrl = href @@ -352,12 +354,13 @@ export function navigateReducer( mutable.previousTree = state.tree mutable.patchedTree = currentTree - mutable.scrollableSegments = scrollableSegments mutable.canonicalUrl = canonicalUrlOverride ? createHrefFromUrl(canonicalUrlOverride) : href mutable.pendingPush = pendingPush + mutable.scrollableSegments = scrollableSegments mutable.hashFragment = hash + mutable.shouldScroll = shouldScroll return handleMutable(state, mutable) } diff --git a/packages/next/src/client/components/router-reducer/router-reducer-types.ts b/packages/next/src/client/components/router-reducer/router-reducer-types.ts index 9a066acb573e6..c7dfe69472ee7 100644 --- a/packages/next/src/client/components/router-reducer/router-reducer-types.ts +++ b/packages/next/src/client/components/router-reducer/router-reducer-types.ts @@ -23,7 +23,8 @@ export type RouterChangeByServerResponse = ( export type RouterNavigate = ( href: string, navigateType: 'push' | 'replace', - forceOptimisticNavigation: boolean + forceOptimisticNavigation: boolean, + shouldScroll: boolean ) => void export interface Mutable { @@ -36,6 +37,7 @@ export interface Mutable { cache?: CacheNode prefetchCache?: AppRouterState['prefetchCache'] hashFragment?: string + shouldScroll?: boolean } export interface ServerActionMutable { @@ -125,6 +127,7 @@ export interface NavigateAction { locationSearch: Location['search'] navigateType: 'push' | 'replace' forceOptimisticNavigation: boolean + shouldScroll: boolean cache: CacheNode mutable: Mutable } diff --git a/packages/next/src/shared/lib/app-router-context.ts b/packages/next/src/shared/lib/app-router-context.ts index b0ca3c444afb6..1977b743e8516 100644 --- a/packages/next/src/shared/lib/app-router-context.ts +++ b/packages/next/src/shared/lib/app-router-context.ts @@ -70,6 +70,7 @@ export type CacheNode = export interface NavigateOptions { /** @internal */ forceOptimisticNavigation?: boolean + scroll: boolean } export interface PrefetchOptions { From c127417dec7e6ada7d8af69d6f1ac7178affa86e Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 29 Jun 2023 15:23:32 +0200 Subject: [PATCH 2/9] pass down link scroll --- packages/next/src/client/components/app-router.tsx | 6 +++--- .../router-reducer/reducers/navigate-reducer.test.tsx | 7 +++++++ .../router-reducer/reducers/server-patch-reducer.test.tsx | 1 + packages/next/src/client/link.tsx | 1 + packages/next/src/shared/lib/app-router-context.ts | 2 +- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 1b31d2bb9d591..93fce31d35ea5 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -289,14 +289,14 @@ function Router({ }) }) }, - replace: (href, options = { scroll: true }) => { + replace: (href, options = {}) => { // @ts-ignore startTransition exists React.startTransition(() => { navigate( href, 'replace', Boolean(options.forceOptimisticNavigation), - options.scroll + options.scroll ?? true ) }) }, @@ -307,7 +307,7 @@ function Router({ href, 'push', Boolean(options.forceOptimisticNavigation), - options.scroll + options.scroll ?? true ) }) }, diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx index 75a4d5abdc96a..e57b080497239 100644 --- a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx +++ b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx @@ -174,6 +174,7 @@ describe('navigateReducer', () => { isExternalUrl: false, locationSearch: '', navigateType: 'push', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -362,6 +363,7 @@ describe('navigateReducer', () => { isExternalUrl: false, locationSearch: '', navigateType: 'push', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -555,6 +557,7 @@ describe('navigateReducer', () => { isExternalUrl, locationSearch: '', navigateType: 'push', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -717,6 +720,7 @@ describe('navigateReducer', () => { isExternalUrl, locationSearch: '', navigateType: 'replace', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -876,6 +880,7 @@ describe('navigateReducer', () => { isExternalUrl: false, locationSearch: '', navigateType: 'push', + shouldScroll: true, forceOptimisticNavigation: true, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -1060,6 +1065,7 @@ describe('navigateReducer', () => { isExternalUrl: false, navigateType: 'push', locationSearch: '', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, @@ -1342,6 +1348,7 @@ describe('navigateReducer', () => { isExternalUrl: false, locationSearch: '', navigateType: 'push', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, diff --git a/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.test.tsx b/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.test.tsx index 6ffcd0548496c..913b1dd8afcc7 100644 --- a/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.test.tsx +++ b/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.test.tsx @@ -502,6 +502,7 @@ describe('serverPatchReducer', () => { isExternalUrl: false, locationSearch: '', navigateType: 'push', + shouldScroll: true, forceOptimisticNavigation: false, cache: { status: CacheStates.LAZY_INITIALIZED, diff --git a/packages/next/src/client/link.tsx b/packages/next/src/client/link.tsx index d17341b43edd2..620deeefd4e19 100644 --- a/packages/next/src/client/link.tsx +++ b/packages/next/src/client/link.tsx @@ -226,6 +226,7 @@ function linkClicked( } else { router[replace ? 'replace' : 'push'](as || href, { forceOptimisticNavigation: !prefetchEnabled, + scroll, }) } } diff --git a/packages/next/src/shared/lib/app-router-context.ts b/packages/next/src/shared/lib/app-router-context.ts index 1977b743e8516..6d47491eb8b9f 100644 --- a/packages/next/src/shared/lib/app-router-context.ts +++ b/packages/next/src/shared/lib/app-router-context.ts @@ -70,7 +70,7 @@ export type CacheNode = export interface NavigateOptions { /** @internal */ forceOptimisticNavigation?: boolean - scroll: boolean + scroll?: boolean } export interface PrefetchOptions { From abc7d3445d603ecbe2341a80508a3e6fc1cad123 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 29 Jun 2023 16:47:22 +0200 Subject: [PATCH 3/9] test: add hash page --- .../navigation/app/hash-change/page.js | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 test/e2e/app-dir/navigation/app/hash-change/page.js diff --git a/test/e2e/app-dir/navigation/app/hash-change/page.js b/test/e2e/app-dir/navigation/app/hash-change/page.js new file mode 100644 index 0000000000000..66bf907e8c355 --- /dev/null +++ b/test/e2e/app-dir/navigation/app/hash-change/page.js @@ -0,0 +1,72 @@ +import React from 'react' +import Link from 'next/link' +import { useRouter } from 'next/router' + +let count = 0 + +const HashChanges = ({ count }) => { + const router = useRouter() + + return ( +
+ + Via Link + + + Via A + + + Page URL + + + Via Empty Hash + + + Go to item 400 + + + Go to name item 400 + + + Go to name item 400 (no scroll) + + + Go to CJK anchor + +

COUNT: {count}

+ {Array.from({ length: 500 }, (x, i) => i + 1).map((i) => { + return ( +
+ {i} +
+ ) + })} + {Array.from({ length: 500 }, (x, i) => i + 1).map((i) => { + return ( +
+ {i} +
+ ) + })} + + Via Top Hash + +
ASPATH: {router.asPath}
+
PATHNAME: {router.pathname}
+
CJK anchor
+
+ ) +} + +HashChanges.getInitialProps = ({ res }) => { + if (res) return { count: 0 } + count += 1 + + return { count } +} + +export default HashChanges From 1f25c93c664a523a50316eefcf33d298442b5d1d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 29 Jun 2023 20:49:40 +0200 Subject: [PATCH 4/9] update --- packages/next/src/client/components/app-router.tsx | 2 +- test/e2e/app-dir/import/next.config.js | 4 +--- test/e2e/app-dir/router-autoscroll/next.config.js | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index d937d70d3ce61..e5b5b515de96b 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -301,7 +301,7 @@ function Router({ ) }) }, - push: (href, options = { scroll: true }) => { + push: (href, options = {}) => { startTransition(() => { navigate( href, diff --git a/test/e2e/app-dir/import/next.config.js b/test/e2e/app-dir/import/next.config.js index 53c8bdda514e2..4ba52ba2c8df6 100644 --- a/test/e2e/app-dir/import/next.config.js +++ b/test/e2e/app-dir/import/next.config.js @@ -1,3 +1 @@ -module.exports = { - strictMode: true, -} +module.exports = {} diff --git a/test/e2e/app-dir/router-autoscroll/next.config.js b/test/e2e/app-dir/router-autoscroll/next.config.js index 53c8bdda514e2..4ba52ba2c8df6 100644 --- a/test/e2e/app-dir/router-autoscroll/next.config.js +++ b/test/e2e/app-dir/router-autoscroll/next.config.js @@ -1,3 +1 @@ -module.exports = { - strictMode: true, -} +module.exports = {} From 0e9f9f51617106446f289fad2c374bd6bb845bcf Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 2 Jul 2023 18:29:26 +0200 Subject: [PATCH 5/9] default scroll --- .../next/src/client/components/app-router.tsx | 5 +- .../router-reducer/handle-mutable.ts | 9 +- .../navigation/app/hash-change/page.js | 109 +++++++++--------- 3 files changed, 65 insertions(+), 58 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index e5b5b515de96b..4f12b6c93dd40 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -57,6 +57,7 @@ import { NotFoundBoundary } from './not-found-boundary' import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache' import { createInfinitePromise } from './infinite-promise' import { NEXT_RSC_UNION_QUERY } from './app-router-headers' +import { Options } from 'selenium-webdriver/chrome' const isServer = typeof window === 'undefined' @@ -297,7 +298,7 @@ function Router({ href, 'replace', Boolean(options.forceOptimisticNavigation), - options.scroll ?? true + !!options.scroll ) }) }, @@ -307,7 +308,7 @@ function Router({ href, 'push', Boolean(options.forceOptimisticNavigation), - options.scroll ?? true + !!options.scroll ) }) }, diff --git a/packages/next/src/client/components/router-reducer/handle-mutable.ts b/packages/next/src/client/components/router-reducer/handle-mutable.ts index 538946641cdae..a1444072f1661 100644 --- a/packages/next/src/client/components/router-reducer/handle-mutable.ts +++ b/packages/next/src/client/components/router-reducer/handle-mutable.ts @@ -9,6 +9,9 @@ export function handleMutable( state: ReadonlyReducerState, mutable: Mutable ): ReducerState { + // shouldScroll is true by default, can override to false. + const shouldScroll = mutable.shouldScroll ?? true + return { buildId: state.buildId, // Set href. @@ -30,13 +33,13 @@ export function handleMutable( }, // All navigation requires scroll and focus management to trigger. focusAndScrollRef: { - apply: mutable.shouldScroll + apply: shouldScroll ? mutable?.scrollableSegments !== undefined ? true : state.focusAndScrollRef.apply : // If shouldScroll is false then we should not apply scroll and focus management. false, - hashFragment: mutable.shouldScroll + hashFragment: shouldScroll ? // Empty hash should trigger default behavior of scrolling layout into view. // #top is handled in layout-router. mutable.hashFragment && mutable.hashFragment !== '' @@ -45,7 +48,7 @@ export function handleMutable( : state.focusAndScrollRef.hashFragment : // If shouldScroll is false then we should not apply scroll and focus management. null, - segmentPaths: mutable.shouldScroll + segmentPaths: shouldScroll ? mutable?.scrollableSegments ?? state.focusAndScrollRef.segmentPaths : // If shouldScroll is false then we should not apply scroll and focus management. [], diff --git a/test/e2e/app-dir/navigation/app/hash-change/page.js b/test/e2e/app-dir/navigation/app/hash-change/page.js index 66bf907e8c355..39ec0bde5cd5f 100644 --- a/test/e2e/app-dir/navigation/app/hash-change/page.js +++ b/test/e2e/app-dir/navigation/app/hash-change/page.js @@ -1,50 +1,66 @@ +'use client' + import React from 'react' import Link from 'next/link' -import { useRouter } from 'next/router' - -let count = 0 +import { useRouter } from 'next/navigation' -const HashChanges = ({ count }) => { +const HashChanges = () => { const router = useRouter() return (
- - Via Link - - - Via A - - - Page URL - - - Via Empty Hash - - - Go to item 400 - - - Go to name item 400 - - - Go to name item 400 (no scroll) - - - Go to CJK anchor - -

COUNT: {count}

- {Array.from({ length: 500 }, (x, i) => i + 1).map((i) => { - return ( -
- {i} -
- ) - })} +
+ + Via Link + +
+ +
+ + Via Empty Hash + +
+ +
+ + Go to item 400 + +
+ +
+ + Go to name item 400 + +
+ +
+ + Go to name item 400 (no scroll) + +
+ +
+ + Go to CJK anchor + +
+ +
+ + Via Top Hash + +
+
ASPATH: {router.asPath}
+
PATHNAME: {router.pathname}
+
CJK anchor
{Array.from({ length: 500 }, (x, i) => i + 1).map((i) => { return (
@@ -52,21 +68,8 @@ const HashChanges = ({ count }) => {
) })} - - Via Top Hash - -
ASPATH: {router.asPath}
-
PATHNAME: {router.pathname}
-
CJK anchor
) } -HashChanges.getInitialProps = ({ res }) => { - if (res) return { count: 0 } - count += 1 - - return { count } -} - export default HashChanges From e6c730900fed2592feb7bf8f39014b60b282385e Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 2 Jul 2023 19:34:36 +0200 Subject: [PATCH 6/9] use default arg --- packages/next/src/client/components/app-router.tsx | 5 ++--- .../client/components/router-reducer/router-reducer-types.ts | 4 ++-- packages/next/src/client/link.tsx | 5 +++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 4f12b6c93dd40..27e8111b6e438 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -57,7 +57,6 @@ import { NotFoundBoundary } from './not-found-boundary' import { findHeadInCache } from './router-reducer/reducers/find-head-in-cache' import { createInfinitePromise } from './infinite-promise' import { NEXT_RSC_UNION_QUERY } from './app-router-headers' -import { Options } from 'selenium-webdriver/chrome' const isServer = typeof window === 'undefined' @@ -298,7 +297,7 @@ function Router({ href, 'replace', Boolean(options.forceOptimisticNavigation), - !!options.scroll + options.scroll ) }) }, @@ -308,7 +307,7 @@ function Router({ href, 'push', Boolean(options.forceOptimisticNavigation), - !!options.scroll + options.scroll ) }) }, diff --git a/packages/next/src/client/components/router-reducer/router-reducer-types.ts b/packages/next/src/client/components/router-reducer/router-reducer-types.ts index c7dfe69472ee7..e688a8e70a5d2 100644 --- a/packages/next/src/client/components/router-reducer/router-reducer-types.ts +++ b/packages/next/src/client/components/router-reducer/router-reducer-types.ts @@ -24,7 +24,7 @@ export type RouterNavigate = ( href: string, navigateType: 'push' | 'replace', forceOptimisticNavigation: boolean, - shouldScroll: boolean + shouldScroll?: boolean ) => void export interface Mutable { @@ -127,7 +127,7 @@ export interface NavigateAction { locationSearch: Location['search'] navigateType: 'push' | 'replace' forceOptimisticNavigation: boolean - shouldScroll: boolean + shouldScroll?: boolean cache: CacheNode mutable: Mutable } diff --git a/packages/next/src/client/link.tsx b/packages/next/src/client/link.tsx index 620deeefd4e19..39fd9ecbd0719 100644 --- a/packages/next/src/client/link.tsx +++ b/packages/next/src/client/link.tsx @@ -217,16 +217,17 @@ function linkClicked( const navigate = () => { // If the router is an NextRouter instance it will have `beforePopState` + const routerScroll = scroll ?? true if ('beforePopState' in router) { router[replace ? 'replace' : 'push'](href, as, { shallow, locale, - scroll, + scroll: routerScroll, }) } else { router[replace ? 'replace' : 'push'](as || href, { forceOptimisticNavigation: !prefetchEnabled, - scroll, + scroll: routerScroll, }) } } From d9251e4d8a78ddfc741e9c1154a2a8fd932a842f Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 2 Jul 2023 20:48:15 +0200 Subject: [PATCH 7/9] update test --- .../reducers/navigate-reducer.test.tsx | 167 ++++++++++++++++++ .../app/{hash-change => hash-changes}/page.js | 0 .../e2e/app-dir/navigation/navigation.test.ts | 11 ++ 3 files changed, 178 insertions(+) rename test/e2e/app-dir/navigation/app/{hash-change => hash-changes}/page.js (100%) diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx index e57b080497239..a5f9580c327c0 100644 --- a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx +++ b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.test.tsx @@ -3,6 +3,7 @@ import type { fetchServerResponse as fetchServerResponseType } from '../fetch-se import type { FlightData } from '../../../../server/app-render/types' const buildId = 'development' + const flightData: FlightData = [ [ 'children', @@ -58,6 +59,9 @@ jest.mock('../fetch-server-response', () => { fetchServerResponse: ( url: URL ): ReturnType => { + if (url.pathname === '/linking' && url.hash === '#hash') { + return Promise.resolve(['', undefined]) + } if (url.pathname === '/linking/about') { return Promise.resolve([flightData, undefined]) } @@ -982,6 +986,169 @@ describe('navigateReducer', () => { expect(newState).toMatchObject(expectedState) }) + it('should apply navigation for scroll', async () => { + const initialTree = getInitialRouterStateTree() + const initialCanonicalUrl = '/linking' + const children = ( + + + Root layout + + ) + const initialParallelRoutes: CacheNode['parallelRoutes'] = new Map([ + [ + 'children', + new Map([ + [ + 'linking', + { + status: CacheStates.READY, + parallelRoutes: new Map([ + [ + 'children', + new Map([ + [ + '__PAGE__', + { + status: CacheStates.READY, + data: null, + subTreeData: <>Linking page, + parallelRoutes: new Map(), + }, + ], + ]), + ], + ]), + data: null, + subTreeData: <>Linking layout level, + }, + ], + ]), + ], + ]) + + const state = createInitialRouterState({ + buildId, + initialTree, + initialHead: null, + initialCanonicalUrl, + children, + initialParallelRoutes, + isServer: false, + location: new URL('/linking', 'https://localhost') as any, + }) + + const state2 = createInitialRouterState({ + buildId, + initialTree, + initialHead: null, + initialCanonicalUrl, + children, + initialParallelRoutes, + isServer: false, + location: new URL('/linking#hash', 'https://localhost') as any, + }) + + const action: NavigateAction = { + type: ACTION_NAVIGATE, + url: new URL('/linking#hash', 'https://localhost'), + isExternalUrl: false, + locationSearch: '', + navigateType: 'push', + shouldScroll: false, // should not scroll + forceOptimisticNavigation: false, + cache: { + status: CacheStates.LAZY_INITIALIZED, + data: null, + subTreeData: null, + parallelRoutes: new Map(), + }, + mutable: {}, + } + + await runPromiseThrowChain(() => navigateReducer(state, action)) + + const newState = await runPromiseThrowChain(() => + navigateReducer(state2, action) + ) + + const expectedState: ReturnType = { + buildId, + prefetchCache: new Map(), + pushRef: { + mpaNavigation: true, + pendingPush: true, + }, + focusAndScrollRef: { + apply: false, + hashFragment: null, + segmentPaths: [], + }, + canonicalUrl: '', + nextUrl: '/linking', + cache: { + status: CacheStates.READY, + data: null, + subTreeData: ( + + + Root layout + + ), + parallelRoutes: new Map([ + [ + 'children', + new Map([ + [ + 'linking', + { + status: CacheStates.READY, + parallelRoutes: new Map([ + [ + 'children', + new Map([ + [ + '__PAGE__', + { + status: CacheStates.READY, + // Real promise is not needed here. + data: null, + parallelRoutes: new Map(), + subTreeData: <>Linking page, + }, + ], + ]), + ], + ]), + // Real promise is not needed here. + data: null, + subTreeData: <>Linking layout level, + }, + ], + ]), + ], + ]), + }, + tree: [ + '', + { + children: [ + 'linking', + { + children: ['__PAGE__', {}], + }, + ], + }, + // TODO-APP: optimistic tree is wrong + undefined, + undefined, + true, + ], + } + + expect(newState).toMatchObject(expectedState) + }) + it('should apply navigation with prefetched data', async () => { const initialTree = getInitialRouterStateTree() const initialCanonicalUrl = '/linking' diff --git a/test/e2e/app-dir/navigation/app/hash-change/page.js b/test/e2e/app-dir/navigation/app/hash-changes/page.js similarity index 100% rename from test/e2e/app-dir/navigation/app/hash-change/page.js rename to test/e2e/app-dir/navigation/app/hash-changes/page.js diff --git a/test/e2e/app-dir/navigation/navigation.test.ts b/test/e2e/app-dir/navigation/navigation.test.ts index 8aaf93dd0bf30..a6e6fab2bcac7 100644 --- a/test/e2e/app-dir/navigation/navigation.test.ts +++ b/test/e2e/app-dir/navigation/navigation.test.ts @@ -85,6 +85,17 @@ createNextDescribe( await checkLink('top', 0) await checkLink('non-existent', 0) }) + + it('should not scroll to hash when scroll={false} is set', async () => { + const browser = await next.browser('/hash-changes') + const curScroll = await browser.eval( + 'document.documentElement.scrollTop' + ) + await browser.elementByCss('#scroll-to-name-item-400-no-scroll').click() + expect(curScroll).toBe( + await browser.eval('document.documentElement.scrollTop') + ) + }) }) describe('hash-with-scroll-offset', () => { From 57446a07f27c563c005dcaaabbe746d758eaf8c1 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 2 Jul 2023 21:17:24 +0200 Subject: [PATCH 8/9] tweak types --- packages/next/src/client/components/app-router.tsx | 6 +++--- .../src/client/components/router-reducer/handle-mutable.ts | 2 +- .../components/router-reducer/router-reducer-types.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 27e8111b6e438..9f6e87fa5943a 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -240,7 +240,7 @@ function Router({ isExternalUrl: isExternalURL(url), locationSearch: location.search, forceOptimisticNavigation, - shouldScroll, + shouldScroll: shouldScroll ?? true, navigateType, cache: createEmptyCacheNode(), mutable: {}, @@ -297,7 +297,7 @@ function Router({ href, 'replace', Boolean(options.forceOptimisticNavigation), - options.scroll + options.scroll ?? true ) }) }, @@ -307,7 +307,7 @@ function Router({ href, 'push', Boolean(options.forceOptimisticNavigation), - options.scroll + options.scroll ?? true ) }) }, diff --git a/packages/next/src/client/components/router-reducer/handle-mutable.ts b/packages/next/src/client/components/router-reducer/handle-mutable.ts index a1444072f1661..79df50fee5d28 100644 --- a/packages/next/src/client/components/router-reducer/handle-mutable.ts +++ b/packages/next/src/client/components/router-reducer/handle-mutable.ts @@ -10,7 +10,7 @@ export function handleMutable( mutable: Mutable ): ReducerState { // shouldScroll is true by default, can override to false. - const shouldScroll = mutable.shouldScroll ?? true + const shouldScroll = mutable.shouldScroll return { buildId: state.buildId, diff --git a/packages/next/src/client/components/router-reducer/router-reducer-types.ts b/packages/next/src/client/components/router-reducer/router-reducer-types.ts index e688a8e70a5d2..c7dfe69472ee7 100644 --- a/packages/next/src/client/components/router-reducer/router-reducer-types.ts +++ b/packages/next/src/client/components/router-reducer/router-reducer-types.ts @@ -24,7 +24,7 @@ export type RouterNavigate = ( href: string, navigateType: 'push' | 'replace', forceOptimisticNavigation: boolean, - shouldScroll?: boolean + shouldScroll: boolean ) => void export interface Mutable { @@ -127,7 +127,7 @@ export interface NavigateAction { locationSearch: Location['search'] navigateType: 'push' | 'replace' forceOptimisticNavigation: boolean - shouldScroll?: boolean + shouldScroll: boolean cache: CacheNode mutable: Mutable } From e0417ef5173e2244866f439ba5923f8bd3b70e80 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 3 Jul 2023 16:23:42 +0200 Subject: [PATCH 9/9] fix mutate --- .../next/src/client/components/router-reducer/handle-mutable.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/client/components/router-reducer/handle-mutable.ts b/packages/next/src/client/components/router-reducer/handle-mutable.ts index 79df50fee5d28..a1444072f1661 100644 --- a/packages/next/src/client/components/router-reducer/handle-mutable.ts +++ b/packages/next/src/client/components/router-reducer/handle-mutable.ts @@ -10,7 +10,7 @@ export function handleMutable( mutable: Mutable ): ReducerState { // shouldScroll is true by default, can override to false. - const shouldScroll = mutable.shouldScroll + const shouldScroll = mutable.shouldScroll ?? true return { buildId: state.buildId,