From ab00f5d705ffa923a4f7b2a650b21121cbaff7f2 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Thu, 6 Jun 2024 16:41:51 -0700 Subject: [PATCH] prevent duplicate RSC fetch when action redirects --- .../reducers/refresh-reducer.ts | 1 + .../reducers/server-action-reducer.ts | 13 ++++--- .../refetch-inactive-parallel-segments.ts | 5 ++- .../parallel-routes-revalidation/app/page.tsx | 2 +- .../app/redirect/page.tsx | 4 ++- .../parallel-routes-revalidation.test.ts | 35 ++++++++++++++++++- 6 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts index 55c624b3959b..b7542244f090 100644 --- a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts +++ b/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 diff --git a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts index 64027eb1aed0..2d580a4a0f66 100644 --- a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts +++ b/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,6 +273,7 @@ export function serverActionReducer( updatedTree: newTree, updatedCache: cache, includeNextUrl: Boolean(nextUrl), + canonicalUrl: mutable.canonicalUrl || state.canonicalUrl, }) mutable.cache = cache @@ -275,16 +281,9 @@ export function serverActionReducer( } mutable.patchedTree = newTree - mutable.canonicalUrl = href - currentTree = newTree } - if (redirectLocation) { - const newHref = createHrefFromUrl(redirectLocation, false) - mutable.canonicalUrl = newHref - } - resolve(actionResult) return handleMutable(state, mutable) diff --git a/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts b/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts index 05c1ddee9c3a..99300a2b2613 100644 --- a/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts +++ b/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 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) diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/page.tsx index 3ef8e7899357..3b35ec775335 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/app/page.tsx +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/page.tsx @@ -8,7 +8,7 @@ export default async function Home() { ).then((res) => res.text()) return ( -
+
Open Revalidate Modal Open Refresh Modal Open Redirect Modal diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/redirect/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/redirect/page.tsx index f451ae9b3b5b..133a96c1208d 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/app/redirect/page.tsx +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/redirect/page.tsx @@ -8,7 +8,9 @@ export default function Page() { redirect('/') }} > - + ) } diff --git a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts index df33fee9ed9b..bb61d0a4a86c 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts +++ b/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) + } + }) + }) }) })