fix: ensure mpa navigation render side effects are only fired once

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<ReducerActions>) {
         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<ReducerActions>): 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<ReducerActions>): 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 <Offscreen>. 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

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<any> {
   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(() =>

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<any> {
   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,
     }
 

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(() =>

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<any> | null
-  globalMutable: { pendingNavigatePath?: string; refresh: () => void }
   actionResultResolved?: boolean
 }
 

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 (
+    <>
+      <Link id="link-to-slow-page" href="/slow-page" ref={slowPageRef}>
+        To /slow-page
+      </Link>
+      <Link id="prefetch-link" href="/hash" ref={prefetchRef}>
+        Prefetch link
+      </Link>
+    </>
+  )
+}