prevent duplicate RSC fetch when action redirects (#66620)

When checking which segment(s) need to be refreshed, we currently
compare the current page URL with the segment's refresh marker.

We should inspect the `mutable.canonicalUrl` value first since that's
the URL we're changing to, followed by `state.canonicalUrl` as a
fallback (indicating that there's no URL change pending). This is
because the server action handler will receive a redirect URL prior to
`location.pathname` being updated, so the router will incorrectly think
it needs to refresh the data for the page we're going to.

Closes NEXT-3500

packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts

@@ -125,6 +125,7 @@ export function refreshReducer(
           updatedTree: newTree,
           updatedCache: cache,
           includeNextUrl,
+          canonicalUrl: mutable.canonicalUrl || state.canonicalUrl,
         })
 
         mutable.cache = cache

packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts

@@ -210,6 +210,11 @@ export function serverActionReducer(
       // Remove cache.data as it has been resolved at this point.
       mutable.inFlightServerAction = null
 
+      if (redirectLocation) {
+        const newHref = createHrefFromUrl(redirectLocation, false)
+        mutable.canonicalUrl = newHref
+      }
+
       for (const flightDataPath of flightData) {
         // FlightDataPath with more than two items means unexpected Flight data was returned
         if (flightDataPath.length !== 4) {
@@ -268,23 +273,17 @@ export function serverActionReducer(
             updatedTree: newTree,
             updatedCache: cache,
             includeNextUrl: Boolean(nextUrl),
+            canonicalUrl: mutable.canonicalUrl || state.canonicalUrl,
           })
 
           mutable.cache = cache
           mutable.prefetchCache = new Map()
         }
 
         mutable.patchedTree = newTree
-        mutable.canonicalUrl = href
-
         currentTree = newTree
       }
 
-      if (redirectLocation) {
-        const newHref = createHrefFromUrl(redirectLocation, false)
-        mutable.canonicalUrl = newHref
-      }
-
       resolve(actionResult)
 
       return handleMutable(state, mutable)

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

@@ -10,6 +10,7 @@ interface RefreshInactiveParallelSegments {
   updatedTree: FlightRouterState
   updatedCache: CacheNode
   includeNextUrl: boolean
+  canonicalUrl: string
 }
 
 /**
@@ -41,6 +42,7 @@ async function refreshInactiveParallelSegmentsImpl({
   includeNextUrl,
   fetchedSegments,
   rootTree = updatedTree,
+  canonicalUrl,
 }: RefreshInactiveParallelSegments & {
   fetchedSegments: Set<string>
   rootTree: FlightRouterState
@@ -50,7 +52,7 @@ async function refreshInactiveParallelSegmentsImpl({
 
   if (
     refetchPath &&
-    refetchPath !== location.pathname + location.search &&
+    refetchPath !== canonicalUrl &&
     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
@@ -94,6 +96,7 @@ async function refreshInactiveParallelSegmentsImpl({
       includeNextUrl,
       fetchedSegments,
       rootTree,
+      canonicalUrl,
     })
 
     fetchPromises.push(parallelFetchPromise)

test/e2e/app-dir/parallel-routes-revalidation/app/page.tsx

@@ -8,7 +8,7 @@ export default async function Home() {
   ).then((res) => res.text())
 
   return (
-    <div>
+    <div id="root-page">
       <Link href="/revalidate-modal">Open Revalidate Modal</Link>
       <Link href="/refresh-modal">Open Refresh Modal</Link>
       <Link href="/redirect-modal">Open Redirect Modal</Link>

test/e2e/app-dir/parallel-routes-revalidation/app/redirect/page.tsx

@@ -8,7 +8,9 @@ export default function Page() {
         redirect('/')
       }}
     >
-      <button type="submit">Redirect</button>
+      <button type="submit" id="redirect-page">
+        Redirect
+      </button>
     </form>
   )
 }

test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts

@@ -2,7 +2,7 @@ import { nextTestSetup } from 'e2e-utils'
 import { check, retry } from 'next-test-utils'
 
 describe('parallel-routes-revalidation', () => {
-  const { next } = nextTestSetup({
+  const { next, isNextStart } = nextTestSetup({
     files: __dirname,
   })
 
@@ -413,5 +413,38 @@ describe('parallel-routes-revalidation', () => {
         )
       })
     })
+
+    it('should not trigger a refresh for the page that is being redirected to', async () => {
+      const rscRequests = []
+      const prefetchRequests = []
+      const browser = await next.browser('/redirect', {
+        beforePageLoad(page) {
+          page.on('request', async (req) => {
+            const headers = await req.allHeaders()
+            if (headers['rsc']) {
+              const pathname = new URL(req.url()).pathname
+
+              if (headers['next-router-prefetch']) {
+                prefetchRequests.push(pathname)
+              } else {
+                rscRequests.push(pathname)
+              }
+            }
+          })
+        },
+      })
+
+      await browser.elementByCss('button').click()
+      await browser.waitForElementByCss('#root-page')
+      await browser.waitForIdleNetwork()
+
+      await retry(async () => {
+        expect(rscRequests.length).toBe(0)
+
+        if (isNextStart) {
+          expect(prefetchRequests.length).toBe(4)
+        }
+      })
+    })
   })
 })