fix: Don't reset shallow URL updates on prefetch (#58297)

## Description

Between 14.0.2-canary.6 and 14.0.2-canary.7, a change was introduced in #56497 that turned the Redux store state into a Promise, rather than a synchronous state update.

This caused the `sync` function -- used to send state updates to the Redux Devtools -- to be recreated on every dispatch, which in turn, by referential instability, caused the `HistoryUpdater` component to re-render and trigger a `history.replaceState` with no particular change, but with the internal `canonicalUrl`.

When an app does a soft/shallow navigation by calling history methods directly (currently the only way to do shallow search params updates in the app router), these changes would have been overwritten by any prefetch (eg: hovering or mounting a Link), which is usually a no-op for the navigation state.

This PR changes the `sync` function to take the state as an argument rather than as a closure. The whole app router state is also unwrapped only once, and fed to the HistoryUpdater. Changes to its contents made by reducers will cause the HistoryUpdater effect to re-run, triggering history updates and a call to the sync function.

## Context

I maintain [`next-usequerystate`](https://github.com/47ng/next-usequerystate), which is used in the Vercel dashboard, and which is impacted by this change (see 
[#388](47ng/nuqs#388)).

## History

@timneutkens introduced the `sync` function and the whole Redux devtools reducer in #39866, with the note:

> a new hook useReducerWithReduxDevtools has been added, we'll probably want to put this behind a compile-time flag when the new router is marked stable but until then it's useful to have it enabled by default (only 
when you have Redux Devtools installed ofcourse). 

If a different direction is needed to keep sending `RENDER_SYNC` actions to Redux devtools, I'll be happy to rework this PR to move the `sync` function into the action queue.

## Changes

- [x] Added e2e test. Requires a `start` mode as prefetch links are disabled in development. Test was verified to fail from next@>=12.0.2-canary.7 without the fix.


Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>

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

@@ -35,7 +35,7 @@ import {
   PrefetchKind,
 } from './router-reducer/router-reducer-types'
 import type {
-  PushRef,
+  AppRouterState,
   ReducerActions,
   RouterChangeByServerResponse,
   RouterNavigate,
@@ -49,6 +49,7 @@ import {
 import {
   useReducerWithReduxDevtools,
   useUnwrapState,
+  type ReduxDevtoolsSyncFn,
 } from './use-reducer-with-devtools'
 import { ErrorBoundary } from './error-boundary'
 import { createInitialRouterState } from './router-reducer/create-initial-router-state'
@@ -110,17 +111,14 @@ function isExternalURL(url: URL) {
 }
 
 function HistoryUpdater({
-  tree,
-  pushRef,
-  canonicalUrl,
+  appRouterState,
   sync,
 }: {
-  tree: FlightRouterState
-  pushRef: PushRef
-  canonicalUrl: string
-  sync: () => void
+  appRouterState: AppRouterState
+  sync: ReduxDevtoolsSyncFn
 }) {
   useInsertionEffect(() => {
+    const { tree, pushRef, canonicalUrl } = appRouterState
     const historyState = {
       ...(process.env.__NEXT_WINDOW_HISTORY_SUPPORT &&
       pushRef.preserveCustomHistoryState
@@ -148,8 +146,8 @@ function HistoryUpdater({
         originalReplaceState(historyState, '', canonicalUrl)
       }
     }
-    sync()
-  }, [tree, pushRef, canonicalUrl, sync])
+    sync(appRouterState)
+  }, [appRouterState, sync])
   return null
 }
 
@@ -589,9 +587,7 @@ function Router({
   return (
     <>
       <HistoryUpdater
-        tree={tree}
-        pushRef={pushRef}
-        canonicalUrl={canonicalUrl}
+        appRouterState={useUnwrapState(reducerState)}
         sync={sync}
       />
       <PathnameContext.Provider value={pathname}>

packages/next/src/client/components/use-reducer-with-devtools.ts

@@ -9,6 +9,8 @@ import {
 } from './router-reducer/router-reducer-types'
 import { ActionQueueContext } from '../../shared/lib/router/action-queue'
 
+export type ReduxDevtoolsSyncFn = (state: AppRouterState) => void
+
 function normalizeRouterState(val: any): any {
   if (val instanceof Map) {
     const obj: { [key: string]: any } = {}
@@ -86,13 +88,13 @@ export function useUnwrapState(state: ReducerState): AppRouterState {
 
 function useReducerWithReduxDevtoolsNoop(
   initialState: AppRouterState
-): [ReducerState, Dispatch<ReducerActions>, () => void] {
+): [ReducerState, Dispatch<ReducerActions>, ReduxDevtoolsSyncFn] {
   return [initialState, () => {}, () => {}]
 }
 
 function useReducerWithReduxDevtoolsImpl(
   initialState: AppRouterState
-): [ReducerState, Dispatch<ReducerActions>, () => void] {
+): [ReducerState, Dispatch<ReducerActions>, ReduxDevtoolsSyncFn] {
   const [state, setState] = React.useState<ReducerState>(initialState)
 
   const actionQueue = useContext(ActionQueueContext)
@@ -149,14 +151,20 @@ function useReducerWithReduxDevtoolsImpl(
     [actionQueue, initialState]
   )
 
-  const sync = useCallback(() => {
+  // Sync is called after a state update in the HistoryUpdater,
+  // for debugging purposes. Since the reducer state may be a Promise,
+  // we let the app router use() it and sync on the resolved value if
+  // something changed.
+  // Using the `state` here would be referentially unstable and cause
+  // undesirable re-renders and history updates.
+  const sync = useCallback<ReduxDevtoolsSyncFn>((resolvedState) => {
     if (devtoolsConnectionRef.current) {
       devtoolsConnectionRef.current.send(
         { type: 'RENDER_SYNC' },
-        normalizeRouterState(state)
+        normalizeRouterState(resolvedState)
       )
     }
-  }, [state])
+  }, [])
 
   return [state, dispatch, sync]
 }

test/e2e/app-dir/navigation/app/search-params/shallow/other/page.js

@@ -0,0 +1,5 @@
+import React from 'react'
+
+export default function Page() {
+  return <p>Prefetch target page</p>
+}

test/e2e/app-dir/navigation/app/search-params/shallow/page.js

@@ -0,0 +1,19 @@
+'use client'
+
+import React from 'react'
+import Link from 'next/link'
+
+export default function Page() {
+  const setShallowSearchParams = React.useCallback(() => {
+    // Maintain history state, but set a shallow search param
+    history.replaceState(history.state, '', '?foo=bar')
+  }, [])
+  return (
+    <>
+      <button onClick={setShallowSearchParams}>Click me first</button>
+      <Link href="/search-params/shallow/other" prefetch>
+        Then hover me
+      </Link>
+    </>
+  )
+}

test/e2e/app-dir/navigation/navigation.test.ts

@@ -55,6 +55,17 @@ createNextDescribe(
         }, 'success')
       })
 
+      it('should not reset shallow url updates on prefetch', async () => {
+        const browser = await next.browser('/search-params/shallow')
+        const button = await browser.elementByCss('button')
+        await button.click()
+        expect(await browser.url()).toMatch(/\?foo=bar$/)
+        const link = await browser.elementByCss('a')
+        await link.hover()
+        // Hovering a prefetch link should keep the URL intact
+        expect(await browser.url()).toMatch(/\?foo=bar$/)
+      })
+
       describe('useParams identity between renders', () => {
         async function runTests(page: string) {
           const browser = await next.browser(page)