diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 2c3c541e02b8e..a8de967825e16 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -33,12 +33,12 @@ import { ACTION_RESTORE, ACTION_SERVER_ACTION, ACTION_SERVER_PATCH, + Mutable, PrefetchKind, ReducerActions, RouterChangeByServerResponse, RouterNavigate, ServerActionDispatcher, - ServerActionMutable, } from './router-reducer/router-reducer-types' import { createHrefFromUrl } from './router-reducer/create-href-from-url' import { @@ -73,7 +73,7 @@ export function getServerActionDispatcher() { return globalServerActionDispatcher } -let globalServerActionMutable: ServerActionMutable['globalMutable'] = { +let globalMutable: Mutable['globalMutable'] = { refresh: () => {}, // noop until the router is initialized } @@ -145,7 +145,7 @@ function useServerActionDispatcher(dispatch: React.Dispatch) { dispatch({ ...actionPayload, type: ACTION_SERVER_ACTION, - mutable: { globalMutable: globalServerActionMutable }, + mutable: { globalMutable }, cache: createEmptyCacheNode(), }) }) @@ -174,7 +174,7 @@ function useChangeByServerResponse( previousTree, overrideCanonicalUrl, cache: createEmptyCacheNode(), - mutable: {}, + mutable: { globalMutable }, }) }) }, @@ -186,7 +186,7 @@ function useNavigate(dispatch: React.Dispatch): RouterNavigate { return useCallback( (href, navigateType, forceOptimisticNavigation, shouldScroll) => { const url = new URL(addBasePath(href), location.href) - globalServerActionMutable.pendingNavigatePath = href + globalMutable.pendingNavigatePath = href return dispatch({ type: ACTION_NAVIGATE, @@ -197,7 +197,7 @@ function useNavigate(dispatch: React.Dispatch): RouterNavigate { shouldScroll: shouldScroll ?? true, navigateType, cache: createEmptyCacheNode(), - mutable: {}, + mutable: { globalMutable }, }) }, [dispatch] @@ -322,7 +322,7 @@ function Router({ dispatch({ type: ACTION_REFRESH, cache: createEmptyCacheNode(), - mutable: {}, + mutable: { globalMutable }, origin: window.location.origin, }) }) @@ -338,7 +338,7 @@ function Router({ dispatch({ type: ACTION_FAST_REFRESH, cache: createEmptyCacheNode(), - mutable: {}, + mutable: { globalMutable }, origin: window.location.origin, }) }) @@ -357,7 +357,7 @@ function Router({ }, [appRouter]) useEffect(() => { - globalServerActionMutable.refresh = appRouter.refresh + globalMutable.refresh = appRouter.refresh }, [appRouter.refresh]) if (process.env.NODE_ENV !== 'production') { @@ -409,11 +409,16 @@ function Router({ // in . At least I hope so. (It will run twice in dev strict mode, // but that's... fine?) if (pushRef.mpaNavigation) { - const location = window.location - if (pushRef.pendingPush) { - location.assign(canonicalUrl) - } else { - location.replace(canonicalUrl) + // if there's a re-render, we don't want to trigger another redirect if one is already in flight to the same URL + if (globalMutable.pendingMpaPath !== canonicalUrl) { + const location = window.location + if (pushRef.pendingPush) { + location.assign(canonicalUrl) + } else { + location.replace(canonicalUrl) + } + + globalMutable.pendingMpaPath = canonicalUrl } // TODO-APP: Should we listen to navigateerror here to catch failed // navigations somehow? And should we call window.stop() if a SPA navigation 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 4ed01fed08c83..fcde9b963e9fd 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 @@ -106,6 +106,10 @@ const getInitialRouterStateTree = (): FlightRouterState => [ true, ] +const globalMutable = { + refresh: () => {}, +} + async function runPromiseThrowChain(fn: any): Promise { try { return await fn() @@ -194,7 +198,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } const newState = await runPromiseThrowChain(() => @@ -438,7 +442,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -633,7 +637,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -792,7 +796,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -948,7 +952,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -1147,7 +1151,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -1317,7 +1321,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -1630,7 +1634,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => navigateReducer(state, action)) @@ -1841,6 +1845,7 @@ describe('navigateReducer', () => { hashFragment: '#hash', pendingPush: true, shouldScroll: true, + globalMutable, }, } @@ -1983,7 +1988,7 @@ describe('navigateReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } const newState = await runPromiseThrowChain(() => diff --git a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.test.tsx b/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.test.tsx index 9ed54e8994d2e..bbf36b1ec538c 100644 --- a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.test.tsx +++ b/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.test.tsx @@ -66,6 +66,10 @@ const getInitialRouterStateTree = (): FlightRouterState => [ true, ] +const globalMutable = { + refresh: () => {}, +} + async function runPromiseThrowChain(fn: any): Promise { try { return await fn() @@ -139,7 +143,7 @@ describe('refreshReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, origin: new URL('/linking', 'https://localhost').origin, } @@ -300,7 +304,7 @@ describe('refreshReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, origin: new URL('/linking', 'https://localhost').origin, } @@ -487,7 +491,7 @@ describe('refreshReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, origin: new URL('/linking', 'https://localhost').origin, } @@ -723,7 +727,7 @@ describe('refreshReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, origin: new URL('/linking', 'https://localhost').origin, } 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 0540c02079cb1..9e7035dc7e819 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 @@ -7,6 +7,10 @@ import type { const buildId = 'development' +const globalMutable = { + refresh: () => {}, +} + jest.mock('../fetch-server-response', () => { const flightData: FlightData = [ [ @@ -184,7 +188,7 @@ describe('serverPatchReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } const newState = await runPromiseThrowChain(() => @@ -375,7 +379,7 @@ describe('serverPatchReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } await runPromiseThrowChain(() => serverPatchReducer(state, action)) @@ -514,7 +518,7 @@ describe('serverPatchReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } const state = createInitialRouterState({ @@ -556,7 +560,7 @@ describe('serverPatchReducer', () => { subTreeData: null, parallelRoutes: new Map(), }, - mutable: {}, + mutable: { globalMutable }, } const newState = await runPromiseThrowChain(() => 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 defbb657c7c42..d9c641c4045b7 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 @@ -38,11 +38,15 @@ export interface Mutable { prefetchCache?: AppRouterState['prefetchCache'] hashFragment?: string shouldScroll?: boolean + globalMutable: { + pendingNavigatePath?: string + pendingMpaPath?: string + refresh: () => void + } } export interface ServerActionMutable extends Mutable { inFlightServerAction?: Promise | null - globalMutable: { pendingNavigatePath?: string; refresh: () => void } actionResultResolved?: boolean } diff --git a/test/e2e/app-dir/navigation/app/mpa-nav-test/page.js b/test/e2e/app-dir/navigation/app/mpa-nav-test/page.js new file mode 100644 index 0000000000000..9a335e2d0995c --- /dev/null +++ b/test/e2e/app-dir/navigation/app/mpa-nav-test/page.js @@ -0,0 +1,38 @@ +'use client' +import Link from 'next/link' +import { useEffect, useRef } from 'react' + +export default function Page() { + const prefetchRef = useRef() + const slowPageRef = useRef() + + useEffect(() => { + function triggerPrefetch() { + const event = new MouseEvent('mouseover', { + view: window, + bubbles: true, + cancelable: true, + }) + + prefetchRef.current.dispatchEvent(event) + console.log('dispatched') + } + + slowPageRef.current.click() + + setInterval(() => { + triggerPrefetch() + }, 1000) + }, []) + + return ( + <> + + To /slow-page + + + Prefetch link + + + ) +} diff --git a/test/e2e/app-dir/navigation/navigation.test.ts b/test/e2e/app-dir/navigation/navigation.test.ts index 67beabf483fe3..b3e1cf19c4261 100644 --- a/test/e2e/app-dir/navigation/navigation.test.ts +++ b/test/e2e/app-dir/navigation/navigation.test.ts @@ -1,5 +1,5 @@ import { createNextDescribe } from 'e2e-utils' -import { check } from 'next-test-utils' +import { check, waitFor } from 'next-test-utils' import type { Request } from 'playwright-chromium' createNextDescribe( @@ -497,6 +497,33 @@ createNextDescribe( .waitForElementByCss('#link-to-app') expect(await browser.url()).toBe(next.url + '/some') }) + + if (!isNextDev) { + // this test is pretty hard to test in playwright, so most of the heavy lifting is in the page component itself + // it triggers a hover on a link to initiate a prefetch request every second, and so we check that + // it doesn't repeatedly initiate the mpa navigation request + it('should not continously initiate a mpa navigation to the same URL when router state changes', async () => { + let requestCount = 0 + const browser = await next.browser('/mpa-nav-test', { + beforePageLoad(page) { + page.on('request', (request) => { + const url = new URL(request.url()) + // skip rsc prefetches + if (url.pathname === '/slow-page' && !url.search) { + requestCount++ + } + }) + }, + }) + + await browser.waitForElementByCss('#link-to-slow-page') + + // wait a few seconds since prefetches are triggered in 1s intervals in the page component + await waitFor(5000) + + expect(requestCount).toBe(1) + }) + } }) describe('nested navigation', () => { @@ -562,7 +589,7 @@ createNextDescribe( ) }) - it('should emit refresh meta tag (peramnent) for redirect page when streaming', async () => { + it('should emit refresh meta tag (permanent) for redirect page when streaming', async () => { const html = await next.render('/redirect/suspense-2') expect(html).toContain( '' diff --git a/test/e2e/app-dir/navigation/pages/slow-page.js b/test/e2e/app-dir/navigation/pages/slow-page.js new file mode 100644 index 0000000000000..ad37357cb6dcf --- /dev/null +++ b/test/e2e/app-dir/navigation/pages/slow-page.js @@ -0,0 +1,13 @@ +export default function Page() { + return 'Hello from slow page' +} + +export async function getServerSideProps({ resolvedUrl }) { + if (!resolvedUrl.includes('?_rsc')) { + // only stall on the navigation, not prefetch + await new Promise((resolve) => setTimeout(resolve, 100000)) + } + return { + props: {}, + } +}