fix interception route refresh behavior with dynamic params

vercel · Apr 3, 2024 · 533c194 · 533c194
1 parent c2c947b
commit 533c194
Show file tree

Hide file tree

Showing 14 changed files with 180 additions and 22 deletions.
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
@@ -27,7 +27,11 @@ export async function refreshInactiveParallelSegments(
   options: RefreshInactiveParallelSegments
 ) {
   const fetchedSegments = new Set<string>()
-  await refreshInactiveParallelSegmentsImpl({ ...options, fetchedSegments })
+  await refreshInactiveParallelSegmentsImpl({
+    ...options,
+    rootTree: options.updatedTree,
+    fetchedSegments,
+  })
 }
 
 async function refreshInactiveParallelSegmentsImpl({
@@ -36,7 +40,11 @@ async function refreshInactiveParallelSegmentsImpl({
   updatedCache,
   includeNextUrl,
   fetchedSegments,
-}: RefreshInactiveParallelSegments & { fetchedSegments: Set<string> }) {
+  rootTree = updatedTree,
+}: RefreshInactiveParallelSegments & {
+  fetchedSegments: Set<string>
+  rootTree: FlightRouterState
+}) {
   const [, parallelRoutes, refetchPathname, refetchMarker] = updatedTree
   const fetchPromises = []
 
@@ -56,7 +64,9 @@ async function refreshInactiveParallelSegmentsImpl({
       // we capture the pathname of the refetch without search params, so that it can be refetched with
       // the "latest" search params when it comes time to actually trigger the fetch (below)
       new URL(refetchPathname + location.search, location.origin),
-      [updatedTree[0], updatedTree[1], updatedTree[2], 'refetch'],
+      // 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'],
       includeNextUrl ? state.nextUrl : null,
       state.buildId
     ).then((fetchResponse) => {
@@ -85,6 +95,7 @@ async function refreshInactiveParallelSegmentsImpl({
       updatedCache,
       includeNextUrl,
       fetchedSegments,
+      rootTree,
     })
 
     fetchPromises.push(parallelFetchPromise)
@@ -104,7 +115,8 @@ export function addRefreshMarkerToActiveParallelSegments(
   pathname: string
 ) {
   const [segment, parallelRoutes, , refetchMarker] = tree
-  if (segment === PAGE_SEGMENT_KEY && refetchMarker !== 'refresh') {
+  // 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[3] = 'refresh'
   }

diff --git a/...lidation/app/refreshing/buttonRefresh.tsx → ...lidation/app/components/RefreshButton.tsx b/...lidation/app/refreshing/buttonRefresh.tsx → ...lidation/app/components/RefreshButton.tsx
@@ -1,7 +1,7 @@
 'use client'
 import { useRouter } from 'next/navigation'
 
-export function Button() {
+export function RefreshButton() {
   const router = useRouter()
 
   return (

diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/components/RevalidateButton.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/components/RevalidateButton.tsx
@@ -0,0 +1,14 @@
+'use client'
+import { revalidateAction } from '../nested-revalidate/@modal/modal/action'
+
+export function RevalidateButton({ id }: { id?: string }) {
+  return (
+    <button
+      id={`revalidate-button${id ? `-${id}` : ''}`}
+      style={{ color: 'orange', padding: '10px' }}
+      onClick={() => revalidateAction()}
+    >
+      Revalidate
+    </button>
+  )
+}
diff --git a/...p-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/(.)login/page.tsx b/...p-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/(.)login/page.tsx
@@ -0,0 +1,22 @@
+import { RefreshButton } from '../../../../components/RefreshButton'
+import { RevalidateButton } from '../../../../components/RevalidateButton'
+
+const getRandom = async () => Math.random()
+
+export default async function Page({ params }) {
+  const someProp = await getRandom()
+
+  return (
+    <dialog open>
+      <div>{params.dynamic}</div>
+      <div style={{ display: 'flex', flexDirection: 'column' }}>
+        <div>
+          <span>Modal Page</span>
+          <span id="modal-random">{someProp}</span>
+        </div>
+        <RefreshButton />
+        <RevalidateButton />
+      </div>
+    </dialog>
+  )
+}
diff --git a/...e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/default.tsx b/...e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/default.tsx
@@ -0,0 +1,3 @@
+export default function Default() {
+  return null
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/layout.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/layout.tsx
@@ -0,0 +1,19 @@
+import Link from 'next/link'
+
+export const dynamic = 'force-dynamic'
+
+export default function Layout({
+  children,
+  modal,
+}: {
+  children: React.ReactNode
+  modal: React.ReactNode
+}) {
+  return (
+    <div>
+      <div>{children}</div>
+      <div>{modal}</div>
+      <Link href="/dynamic-refresh/foo/other">Go to Other Page</Link>
+    </div>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/login/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/login/page.tsx
@@ -0,0 +1,11 @@
+import { RefreshButton } from '../../../components/RefreshButton'
+
+export default function Page() {
+  return (
+    <>
+      <span>Login Page</span>
+      <RefreshButton />
+      Random Number: <span id="login-page-random">{Math.random()}</span>
+    </>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/other/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/other/page.tsx
@@ -0,0 +1,13 @@
+import { RefreshButton } from '../../../components/RefreshButton'
+import { RevalidateButton } from '../../../components/RevalidateButton'
+
+export default function Page() {
+  return (
+    <div>
+      <div>Other Page</div>
+      <div id="other-page-random">{Math.random()}</div>
+      <RefreshButton />
+      <RevalidateButton id="other" />
+    </div>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/page.tsx
@@ -0,0 +1,14 @@
+import Link from 'next/link'
+
+export default function Home() {
+  return (
+    <main>
+      <Link href="/dynamic-refresh/foo/login">
+        <button>Login button</button>
+      </Link>
+      <div>
+        Random # from Root Page: <span id="random-number">{Math.random()}</span>
+      </div>
+    </main>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/@modal/(.)login/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/@modal/(.)login/page.tsx
@@ -1,4 +1,5 @@
-import { Button } from '../../buttonRefresh'
+import { RefreshButton } from '../../../components/RefreshButton'
+import { RevalidateButton } from '../../../components/RevalidateButton'
 
 const getRandom = async () => Math.random()
 
@@ -12,7 +13,8 @@ export default async function Page() {
           <span>Modal Page</span>
           <span id="modal-random">{someProp}</span>
         </div>
-        <Button />
+        <RefreshButton />
+        <RevalidateButton />
       </div>
     </dialog>
   )

diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/login/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/login/page.tsx
@@ -1,10 +1,10 @@
-import { Button } from '../buttonRefresh'
+import { RefreshButton } from '../../components/RefreshButton'
 
 export default function Page() {
   return (
     <>
       <span>Login Page</span>
-      <Button />
+      <RefreshButton />
       Random Number: <span id="login-page-random">{Math.random()}</span>
     </>
   )

diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/other/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/other/page.tsx
@@ -1,8 +1,13 @@
+import { RefreshButton } from '../../components/RefreshButton'
+import { RevalidateButton } from '../../components/RevalidateButton'
+
 export default function Page() {
   return (
     <div>
       <div>Other Page</div>
       <div id="other-page-random">{Math.random()}</div>
+      <RefreshButton />
+      <RevalidateButton id="other" />
     </div>
   )
 }
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
@@ -158,21 +158,42 @@ createNextDescribe(
       })
     })
 
-    describe('router.refresh', () => {
+    describe.each([
+      { basePath: '/refreshing', label: 'regular' },
+      { basePath: '/dynamic-refresh/foo', label: 'dynamic' },
+    ])('router.refresh ($label)', ({ basePath }) => {
       it('should correctly refresh data for the intercepted route and previously active page slot', async () => {
-        const browser = await next.browser('/refreshing')
-        const initialRandomNumber = await browser.elementById('random-number')
+        const browser = await next.browser(basePath)
+        let initialRandomNumber = await browser.elementById('random-number')
 
-        await browser.elementByCss("[href='/refreshing/login']").click()
+        await browser.elementByCss(`[href='${basePath}/login']`).click()
 
         // interception modal should be visible
-        const initialModalRandomNumber = await browser
+        let initialModalRandomNumber = await browser
           .elementById('modal-random')
           .text()
 
         // trigger a refresh
         await browser.elementById('refresh-button').click()
 
+        await retry(async () => {
+          const newRandomNumber = await browser
+            .elementById('random-number')
+            .text()
+          const newModalRandomNumber = await browser
+            .elementById('modal-random')
+            .text()
+          expect(initialRandomNumber).not.toBe(newRandomNumber)
+          expect(initialModalRandomNumber).not.toBe(newModalRandomNumber)
+
+          // reset the initial values to be the new values, so that we can verify the revalidate case below.
+          initialRandomNumber = newRandomNumber
+          initialModalRandomNumber = newModalRandomNumber
+        })
+
+        // trigger a revalidate
+        await browser.elementById('revalidate-button').click()
+
         await retry(async () => {
           const newRandomNumber = await browser
             .elementById('random-number')
@@ -205,26 +226,46 @@ createNextDescribe(
         })
       })
 
-      it('should correctly refresh data for previously intercepted modal and active page slot', async () => {
-        const browser = await next.browser('/refreshing')
+      it('should correctly refresh data for previously intercepted modal and active page slot (%s)', async () => {
+        const browser = await next.browser(basePath)
 
-        await browser.elementByCss("[href='/refreshing/login']").click()
+        await browser.elementByCss(`[href='${basePath}/login']`).click()
 
         // interception modal should be visible
-        const initialModalRandomNumber = await browser
+        let initialModalRandomNumber = await browser
           .elementById('modal-random')
           .text()
 
-        await browser.elementByCss("[href='/refreshing/other']").click()
+        await browser.elementByCss(`[href='${basePath}/other']`).click()
         // data for the /other page should be visible
 
-        const initialOtherPageRandomNumber = await browser
+        let initialOtherPageRandomNumber = await browser
           .elementById('other-page-random')
           .text()
 
         // trigger a refresh
         await browser.elementById('refresh-button').click()
 
+        await retry(async () => {
+          const newModalRandomNumber = await browser
+            .elementById('modal-random')
+            .text()
+
+          const newOtherPageRandomNumber = await browser
+            .elementById('other-page-random')
+            .text()
+          expect(initialModalRandomNumber).not.toBe(newModalRandomNumber)
+          expect(initialOtherPageRandomNumber).not.toBe(
+            newOtherPageRandomNumber
+          )
+          // reset the initial values to be the new values, so that we can verify the revalidate case below.
+          initialOtherPageRandomNumber = newOtherPageRandomNumber
+          initialModalRandomNumber = newModalRandomNumber
+        })
+
+        // trigger a revalidate
+        await browser.elementById('revalidate-button').click()
+
         await retry(async () => {
           const newModalRandomNumber = await browser
             .elementById('modal-random')

diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json
@@ -2271,8 +2271,10 @@
         "parallel-routes-revalidation should handle a redirect action when called in a slot",
         "parallel-routes-revalidation should handle router.refresh() when called in a slot",
         "parallel-routes-revalidation should not trigger full page when calling router.refresh() on an intercepted route",
-        "parallel-routes-revalidation router.refresh should correctly refresh data for the intercepted route and previously active page slot",
-        "parallel-routes-revalidation router.refresh should correctly refresh data for previously intercepted modal and active page slot",
+        "parallel-routes-revalidation router.refresh (dynamic) should correctly refresh data for the intercepted route and previously active page slot",
+        "parallel-routes-revalidation router.refresh (dynamic) should correctly refresh data for previously intercepted modal and active page slot (%s)",
+        "parallel-routes-revalidation router.refresh (regular) should correctly refresh data for the intercepted route and previously active page slot",
+        "parallel-routes-revalidation router.refresh (regular) should correctly refresh data for previously intercepted modal and active page slot (%s)",
         "parallel-routes-revalidation server action revalidation handles refreshing when multiple parallel slots are active"
       ],
       "pending": [],