fix action revalidation within a parallel slot

vercel · Dec 15, 2023 · c5867f3 · c5867f3
1 parent a0c68c5
commit c5867f3
Show file tree

Hide file tree

Showing 22 changed files with 347 additions and 32 deletions.
diff --git a/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.test.tsx b/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.test.tsx
@@ -3,7 +3,7 @@ import type {
   FlightData,
   FlightRouterState,
 } from '../../../server/app-render/types'
-import { applyRouterStatePatchToTree } from './apply-router-state-patch-to-tree'
+import { applyRouterStatePatchToTreeSkipDefault } from './apply-router-state-patch-to-tree'
 
 const getInitialRouterStateTree = (): FlightRouterState => [
   '',
@@ -55,7 +55,7 @@ describe('applyRouterStatePatchToTree', () => {
     const [treePatch /*, cacheNodeSeedData, head*/] = flightDataPath.slice(-3)
     const flightSegmentPath = flightDataPath.slice(0, -4)
 
-    const newRouterStateTree = applyRouterStatePatchToTree(
+    const newRouterStateTree = applyRouterStatePatchToTreeSkipDefault(
       ['', ...flightSegmentPath],
       initialRouterStateTree,
       treePatch

diff --git a/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts b/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts
@@ -10,14 +10,18 @@ import { matchSegment } from '../match-segments'
  */
 function applyPatch(
   initialTree: FlightRouterState,
-  patchTree: FlightRouterState
+  patchTree: FlightRouterState,
+  applyPatchToDefaultSegment: boolean = false
 ): FlightRouterState {
   const [initialSegment, initialParallelRoutes] = initialTree
   const [patchSegment, patchParallelRoutes] = patchTree
 
-  // if the applied patch segment is __DEFAULT__ then we can ignore it and return the initial tree
+  // if the applied patch segment is __DEFAULT__ then it can be ignored in favor of the initial tree
   // this is because the __DEFAULT__ segment is used as a placeholder on navigation
+  // however, there are cases where we _do_ want to apply the patch to the default segment,
+  // such as when revalidating the router cache with router.refresh/revalidatePath
   if (
+    !applyPatchToDefaultSegment &&
     patchSegment === DEFAULT_SEGMENT_KEY &&
     initialSegment !== DEFAULT_SEGMENT_KEY
   ) {
@@ -32,7 +36,8 @@ function applyPatch(
       if (isInPatchTreeParallelRoutes) {
         newParallelRoutes[key] = applyPatch(
           initialParallelRoutes[key],
-          patchParallelRoutes[key]
+          patchParallelRoutes[key],
+          applyPatchToDefaultSegment
         )
       } else {
         newParallelRoutes[key] = initialParallelRoutes[key]
@@ -67,19 +72,21 @@ function applyPatch(
   return patchTree
 }
 
-/**
- * Apply the router state from the Flight response. Creates a new router state tree.
- */
-export function applyRouterStatePatchToTree(
+function applyRouterStatePatchToTreeImpl(
   flightSegmentPath: FlightSegmentPath,
   flightRouterState: FlightRouterState,
-  treePatch: FlightRouterState
+  treePatch: FlightRouterState,
+  applyPatchDefaultSegment: boolean = false
 ): FlightRouterState | null {
   const [segment, parallelRoutes, , , isRootLayout] = flightRouterState
 
   // Root refresh
   if (flightSegmentPath.length === 1) {
-    const tree: FlightRouterState = applyPatch(flightRouterState, treePatch)
+    const tree: FlightRouterState = applyPatch(
+      flightRouterState,
+      treePatch,
+      applyPatchDefaultSegment
+    )
 
     return tree
   }
@@ -95,12 +102,17 @@ export function applyRouterStatePatchToTree(
 
   let parallelRoutePatch
   if (lastSegment) {
-    parallelRoutePatch = applyPatch(parallelRoutes[parallelRouteKey], treePatch)
+    parallelRoutePatch = applyPatch(
+      parallelRoutes[parallelRouteKey],
+      treePatch,
+      applyPatchDefaultSegment
+    )
   } else {
-    parallelRoutePatch = applyRouterStatePatchToTree(
+    parallelRoutePatch = applyRouterStatePatchToTreeImpl(
       flightSegmentPath.slice(2),
       parallelRoutes[parallelRouteKey],
-      treePatch
+      treePatch,
+      applyPatchDefaultSegment
     )
 
     if (parallelRoutePatch === null) {
@@ -123,3 +135,39 @@ export function applyRouterStatePatchToTree(
 
   return tree
 }
+
+/**
+ * Apply the router state from the Flight response to the tree, including default segments.
+ * Useful for patching the router cache when we expect to revalidate the full tree, such as with router.refresh or revalidatePath.
+ * Creates a new router state tree.
+ */
+export function applyRouterStatePatchToFullTree(
+  flightSegmentPath: FlightSegmentPath,
+  flightRouterState: FlightRouterState,
+  treePatch: FlightRouterState
+): FlightRouterState | null {
+  return applyRouterStatePatchToTreeImpl(
+    flightSegmentPath,
+    flightRouterState,
+    treePatch,
+    true
+  )
+}
+
+/**
+ * Apply the router state from the Flight response, but skip patching default segments.
+ * Useful for patching the router cache when navigating, where we persist the existing default segment if there isn't a new one.
+ * Creates a new router state tree.
+ */
+export function applyRouterStatePatchToTreeSkipDefault(
+  flightSegmentPath: FlightSegmentPath,
+  flightRouterState: FlightRouterState,
+  treePatch: FlightRouterState
+): FlightRouterState | null {
+  return applyRouterStatePatchToTreeImpl(
+    flightSegmentPath,
+    flightRouterState,
+    treePatch,
+    false
+  )
+}
diff --git a/packages/next/src/client/components/router-reducer/create-router-cache-key.ts b/packages/next/src/client/components/router-reducer/create-router-cache-key.ts
@@ -5,9 +5,17 @@ export function createRouterCacheKey(
   segment: Segment,
   withoutSearchParameters: boolean = false
 ) {
-  return Array.isArray(segment)
-    ? `${segment[0]}|${segment[1]}|${segment[2]}`.toLowerCase()
-    : withoutSearchParameters && segment.startsWith(PAGE_SEGMENT_KEY)
-    ? PAGE_SEGMENT_KEY
-    : segment
+  // if the segment is an array, it means it's a dynamic segment
+  // for example, ['lang', 'en', 'd']. We need to convert it to a string to store it as a cache node key.
+  if (Array.isArray(segment)) {
+    return `${segment[0]}|${segment[1]}|${segment[2]}`.toLowerCase()
+  }
+
+  // Page segments might have search parameters, ie __PAGE__?foo=bar
+  // When `withoutSearchParameters` is true, we only want to return the page segment
+  if (withoutSearchParameters && segment.startsWith(PAGE_SEGMENT_KEY)) {
+    return PAGE_SEGMENT_KEY
+  }
+
+  return segment
 }
diff --git a/packages/next/src/client/components/router-reducer/reducers/fast-refresh-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/fast-refresh-reducer.ts
@@ -1,6 +1,6 @@
 import { fetchServerResponse } from '../fetch-server-response'
 import { createHrefFromUrl } from '../create-href-from-url'
-import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
+import { applyRouterStatePatchToTreeSkipDefault } from '../apply-router-state-patch-to-tree'
 import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
 import type {
   ReadonlyReducerState,
@@ -63,7 +63,7 @@ function fastRefreshReducerImpl(
 
         // Given the path can only have two items the items are only the router state and rsc for the root.
         const [treePatch] = flightDataPath
-        const newTree = applyRouterStatePatchToTree(
+        const newTree = applyRouterStatePatchToTreeSkipDefault(
           // TODO-APP: remove ''
           [''],
           currentTree,

diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts
@@ -8,7 +8,7 @@ import type { FetchServerResponseResult } from '../fetch-server-response'
 import { createHrefFromUrl } from '../create-href-from-url'
 import { invalidateCacheBelowFlightSegmentPath } from '../invalidate-cache-below-flight-segmentpath'
 import { fillCacheWithDataProperty } from '../fill-cache-with-data-property'
-import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
+import { applyRouterStatePatchToTreeSkipDefault } from '../apply-router-state-patch-to-tree'
 import { shouldHardNavigate } from '../should-hard-navigate'
 import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
 import type {
@@ -187,7 +187,7 @@ function navigateReducer_noPPR(
         const flightSegmentPathWithLeadingEmpty = ['', ...flightSegmentPath]
 
         // Create new tree based on the flightSegmentPath and router state patch
-        let newTree = applyRouterStatePatchToTree(
+        let newTree = applyRouterStatePatchToTreeSkipDefault(
           // TODO-APP: remove ''
           flightSegmentPathWithLeadingEmpty,
           currentTree,
@@ -197,7 +197,7 @@ function navigateReducer_noPPR(
         // If the tree patch can't be applied to the current tree then we use the tree at time of prefetch
         // TODO-APP: This should instead fill in the missing pieces in `currentTree` with the data from `treeAtTimeOfPrefetch`, then apply the patch.
         if (newTree === null) {
-          newTree = applyRouterStatePatchToTree(
+          newTree = applyRouterStatePatchToTreeSkipDefault(
             // TODO-APP: remove ''
             flightSegmentPathWithLeadingEmpty,
             treeAtTimeOfPrefetch,
@@ -381,7 +381,7 @@ function navigateReducer_PPR(
         const flightSegmentPathWithLeadingEmpty = ['', ...flightSegmentPath]
 
         // Create new tree based on the flightSegmentPath and router state patch
-        let newTree = applyRouterStatePatchToTree(
+        let newTree = applyRouterStatePatchToTreeSkipDefault(
           // TODO-APP: remove ''
           flightSegmentPathWithLeadingEmpty,
           currentTree,
@@ -391,7 +391,7 @@ function navigateReducer_PPR(
         // If the tree patch can't be applied to the current tree then we use the tree at time of prefetch
         // TODO-APP: This should instead fill in the missing pieces in `currentTree` with the data from `treeAtTimeOfPrefetch`, then apply the patch.
         if (newTree === null) {
-          newTree = applyRouterStatePatchToTree(
+          newTree = applyRouterStatePatchToTreeSkipDefault(
             // TODO-APP: remove ''
             flightSegmentPathWithLeadingEmpty,
             treeAtTimeOfPrefetch,

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
@@ -1,6 +1,6 @@
 import { fetchServerResponse } from '../fetch-server-response'
 import { createHrefFromUrl } from '../create-href-from-url'
-import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
+import { applyRouterStatePatchToFullTree } from '../apply-router-state-patch-to-tree'
 import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
 import type {
   Mutable,
@@ -61,7 +61,7 @@ export function refreshReducer(
 
         // Given the path can only have two items the items are only the router state and rsc for the root.
         const [treePatch] = flightDataPath
-        const newTree = applyRouterStatePatchToTree(
+        const newTree = applyRouterStatePatchToFullTree(
           // TODO-APP: remove ''
           [''],
           currentTree,

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
@@ -31,7 +31,7 @@ import type {
 import { addBasePath } from '../../../add-base-path'
 import { createHrefFromUrl } from '../create-href-from-url'
 import { handleExternalUrl } from './navigate-reducer'
-import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
+import { applyRouterStatePatchToFullTree } from '../apply-router-state-patch-to-tree'
 import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
 import type { CacheNode } from '../../../../shared/lib/app-router-context.shared-runtime'
 import { handleMutable } from '../handle-mutable'
@@ -216,7 +216,7 @@ export function serverActionReducer(
 
         // Given the path can only have two items the items are only the router state and rsc for the root.
         const [treePatch] = flightDataPath
-        const newTree = applyRouterStatePatchToTree(
+        const newTree = applyRouterStatePatchToFullTree(
           // TODO-APP: remove ''
           [''],
           currentTree,

diff --git a/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.ts
@@ -1,5 +1,5 @@
 import { createHrefFromUrl } from '../create-href-from-url'
-import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
+import { applyRouterStatePatchToTreeSkipDefault } from '../apply-router-state-patch-to-tree'
 import { isNavigatingToNewRootLayout } from '../is-navigating-to-new-root-layout'
 import type {
   ServerPatchAction,
@@ -41,7 +41,7 @@ export function serverPatchReducer(
     const flightSegmentPath = flightDataPath.slice(0, -4)
 
     const [treePatch] = flightDataPath.slice(-3, -2)
-    const newTree = applyRouterStatePatchToTree(
+    const newTree = applyRouterStatePatchToTreeSkipDefault(
       // TODO-APP: remove ''
       ['', ...flightSegmentPath],
       currentTree,

diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/default.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/default.tsx
@@ -0,0 +1,3 @@
+export default function Default() {
+  return null
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return null
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/refresh-modal/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/refresh-modal/page.tsx
@@ -0,0 +1,21 @@
+'use client'
+import Link from 'next/link'
+import { useRouter } from 'next/navigation'
+import React from 'react'
+
+export default function Page() {
+  const router = useRouter()
+
+  return (
+    <dialog open>
+      <h1>Modal</h1>
+
+      <br />
+
+      <button onClick={() => router.refresh()} id="refresh-router">
+        Refresh Router
+      </button>
+      <Link href="/">Close</Link>
+    </dialog>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/revalidate-modal/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/revalidate-modal/page.tsx
@@ -0,0 +1,30 @@
+import Link from 'next/link'
+
+import { revalidatePath } from 'next/cache'
+import { addData } from '../../actions'
+
+export default function Page() {
+  async function createItem() {
+    'use server'
+
+    await addData(new Date().toISOString())
+
+    revalidatePath('/', 'layout')
+  }
+
+  return (
+    <dialog open>
+      <h1>Modal</h1>
+
+      <br />
+
+      <form action={createItem}>
+        <button type="submit" className="button" id="create-entry">
+          Create New Item
+        </button>
+      </form>
+
+      <Link href="/">Close</Link>
+    </dialog>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/@interception/(.)redirect-modal/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/@interception/(.)redirect-modal/page.tsx
@@ -0,0 +1,14 @@
+import { RedirectForm } from '../../components/RedirectForm'
+import { redirectAction } from '../../actions'
+
+export default function Page() {
+  return (
+    <dialog open>
+      <h1>Modal</h1>
+
+      <br />
+
+      <RedirectForm action={redirectAction} />
+    </dialog>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/@interception/default.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/@interception/default.tsx
@@ -0,0 +1,3 @@
+export default function Default() {
+  return null
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/actions.ts b/test/e2e/app-dir/parallel-routes-revalidation/app/actions.ts
@@ -0,0 +1,23 @@
+'use server'
+import { redirect } from 'next/navigation'
+
+const data = []
+
+export async function addData(newData: string) {
+  // sleep 1s
+  await new Promise((resolve) => setTimeout(resolve, 1000))
+  data.push(newData)
+}
+
+export async function getData() {
+  // sleep 1s
+  await new Promise((resolve) => setTimeout(resolve, 1000))
+  return data
+}
+
+export async function redirectAction() {
+  'use server'
+  console.log('redirecting...')
+  await new Promise((res) => setTimeout(res, 1000))
+  redirect('/')
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/components/RedirectForm.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/components/RedirectForm.tsx
@@ -0,0 +1,11 @@
+'use client'
+
+export function RedirectForm({ action }: { action: () => Promise<void> }) {
+  return (
+    <form action={action}>
+      <button type="submit" className="button" id="redirect">
+        Redirect to Home
+      </button>
+    </form>
+  )
+}
diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/default.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/default.tsx
@@ -0,0 +1 @@
+export { default } from './page'