fix incorrect refresh request when basePath is set (#64589)

`initialCanonicalUrl` differs from the `canonicalUrl` that gets set on
the client, such as when there's a basePath set.

This hoists the `canonicalUrl` construction up so we can re-use it when
adding refetch markers to the tree.

This also renames `pathname` -> `path` since it also includes search
params. I added a test to confirm no extra erroneous fetches happened in
both cases.

Fixes #64584


Closes NEXT-3130

Author: ztanner

packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts

@@ -80,7 +80,7 @@ export function applyRouterStatePatchToTree(
   flightSegmentPath: FlightSegmentPath,
   flightRouterState: FlightRouterState,
   treePatch: FlightRouterState,
-  pathname: string
+  path: string
 ): FlightRouterState | null {
   const [segment, parallelRoutes, url, refetch, isRootLayout] =
     flightRouterState
@@ -93,7 +93,7 @@ export function applyRouterStatePatchToTree(
       flightSegmentPath
     )
 
-    addRefreshMarkerToActiveParallelSegments(tree, pathname)
+    addRefreshMarkerToActiveParallelSegments(tree, path)
 
     return tree
   }
@@ -119,7 +119,7 @@ export function applyRouterStatePatchToTree(
       flightSegmentPath.slice(2),
       parallelRoutes[parallelRouteKey],
       treePatch,
-      pathname
+      path
     )
 
     if (parallelRoutePatch === null) {
@@ -142,7 +142,7 @@ export function applyRouterStatePatchToTree(
     tree[4] = true
   }
 
-  addRefreshMarkerToActiveParallelSegments(tree, pathname)
+  addRefreshMarkerToActiveParallelSegments(tree, path)
 
   return tree
 }

packages/next/src/client/components/router-reducer/create-initial-router-state.ts

@@ -49,7 +49,15 @@ export function createInitialRouterState({
     loading: initialSeedData[3],
   }
 
-  addRefreshMarkerToActiveParallelSegments(initialTree, initialCanonicalUrl)
+  const canonicalUrl =
+    // location.href is read as the initial value for canonicalUrl in the browser
+    // This is safe to do as canonicalUrl can't be rendered, it's only used to control the history updates in the useEffect further down in this file.
+    location
+      ? // window.location does not have the same type as URL but has all the fields createHrefFromUrl needs.
+        createHrefFromUrl(location)
+      : initialCanonicalUrl
+
+  addRefreshMarkerToActiveParallelSegments(initialTree, canonicalUrl)
 
   const prefetchCache = new Map<string, PrefetchCacheEntry>()
 
@@ -82,13 +90,7 @@ export function createInitialRouterState({
       hashFragment: null,
       segmentPaths: [],
     },
-    canonicalUrl:
-      // location.href is read as the initial value for canonicalUrl in the browser
-      // This is safe to do as canonicalUrl can't be rendered, it's only used to control the history updates in the useEffect further down in this file.
-      location
-        ? // window.location does not have the same type as URL but has all the fields createHrefFromUrl needs.
-          createHrefFromUrl(location)
-        : initialCanonicalUrl,
+    canonicalUrl,
     nextUrl:
       // the || operator is intentional, the pathname can be an empty string
       (extractPathFromFlightRouterState(initialTree) || location?.pathname) ??

packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts

@@ -45,23 +45,23 @@ async function refreshInactiveParallelSegmentsImpl({
   fetchedSegments: Set<string>
   rootTree: FlightRouterState
 }) {
-  const [, parallelRoutes, refetchPathname, refetchMarker] = updatedTree
+  const [, parallelRoutes, refetchPath, refetchMarker] = updatedTree
   const fetchPromises = []
 
   if (
-    refetchPathname &&
-    refetchPathname !== location.pathname &&
+    refetchPath &&
+    refetchPath !== location.pathname + location.search &&
     refetchMarker === 'refresh' &&
     // it's possible for the tree to contain multiple segments that contain data at the same URL
     // we keep track of them so we can dedupe the requests
-    !fetchedSegments.has(refetchPathname)
+    !fetchedSegments.has(refetchPath)
   ) {
-    fetchedSegments.add(refetchPathname) // Mark this URL as fetched
+    fetchedSegments.add(refetchPath) // Mark this URL as fetched
 
     // Eagerly kick off the fetch for the refetch path & the parallel routes. This should be fine to do as they each operate
     // independently on their own cache nodes, and `applyFlightData` will copy anything it doesn't care about from the existing cache.
     const fetchPromise = fetchServerResponse(
-      new URL(refetchPathname, location.origin),
+      new URL(refetchPath, location.origin),
       // refetch from the root of the updated tree, otherwise it will be scoped to the current segment
       // and might not contain the data we need to patch in interception route data (such as dynamic params from a previous segment)
       [rootTree[0], rootTree[1], rootTree[2], 'refetch'],
@@ -110,16 +110,16 @@ async function refreshInactiveParallelSegmentsImpl({
  */
 export function addRefreshMarkerToActiveParallelSegments(
   tree: FlightRouterState,
-  pathname: string
+  path: string
 ) {
   const [segment, parallelRoutes, , refetchMarker] = tree
   // a page segment might also contain concatenated search params, so we do a partial match on the key
   if (segment.includes(PAGE_SEGMENT_KEY) && refetchMarker !== 'refresh') {
-    tree[2] = pathname
+    tree[2] = path
     tree[3] = 'refresh'
   }
 
   for (const key in parallelRoutes) {
-    addRefreshMarkerToActiveParallelSegments(parallelRoutes[key], pathname)
+    addRefreshMarkerToActiveParallelSegments(parallelRoutes[key], path)
   }
 }

test/e2e/app-dir/app-basepath/app/refresh/page.js

@@ -0,0 +1,16 @@
+'use client'
+
+import { useRouter } from 'next/navigation'
+
+export default function Page() {
+  const router = useRouter()
+  return (
+    <button
+      onClick={() => {
+        router.refresh()
+      }}
+    >
+      Refresh
+    </button>
+  )
+}

test/e2e/app-dir/app-basepath/index.test.ts

@@ -64,5 +64,32 @@ createNextDescribe(
         expect(await browser.url()).toBe(`${next.url}/base/dynamic/dest`)
       })
     })
+
+    it.each(['/base/refresh', '/base/refresh?foo=bar'])(
+      `should only make a single RSC call to the current page (%s)`,
+      async (path) => {
+        let rscRequests = []
+        const browser = await next.browser(path, {
+          beforePageLoad(page) {
+            page.on('request', (request) => {
+              return request.allHeaders().then((headers) => {
+                if (
+                  headers['RSC'.toLowerCase()] === '1' &&
+                  // Prefetches also include `RSC`
+                  headers['Next-Router-Prefetch'.toLowerCase()] !== '1'
+                ) {
+                  rscRequests.push(request.url())
+                }
+              })
+            })
+          },
+        })
+        await browser.elementByCss('button').click()
+        await retry(async () => {
+          expect(rscRequests.length).toBe(1)
+          expect(rscRequests[0]).toContain(`${next.url}${path}`)
+        })
+      }
+    )
   }
 )