Fix parallel routes with server actions / revalidating router cache (#…

…59585)

### What?
There are a bunch of different bugs caused by the same underlying issue,
but the common thread is that performing any sort of router cache update
(either through `router.refresh()`, `revalidatePath()`, or `redirect()`)
inside of a parallel route would break the router preventing subsequent
actions, and not resolve any pending state such as from `useFormState`.

### Why?
`applyPatch` is responsible for taking an update response from the
server and merging it into the client router cache. However, there's
specific bailout logic to skip over applying the patch to a
`__DEFAULT__` segment (which corresponds with a `default.tsx` page).
When the router detects a cache node that is expected to be rendered on
the page but contains no data, the router will trigger a lazy fetch to
retrieve the data that's expected to be there
([ref](https://github.com/vercel/next.js/blob/5adacb69126e0fd7dff7ebd45278c0dfd42f6116/packages/next/src/client/components/layout-router.tsx#L359-L370))
and then update the router cache once the data resolves
([ref](https://github.com/vercel/next.js/blob/5adacb69126e0fd7dff7ebd45278c0dfd42f6116/packages/next/src/client/components/layout-router.tsx#L399-L404)).

This is causing the router to get stuck in a loop: it'll fetch the data
for the cache node, send the data to the router reducer to merge it into
the existing cache nodes, skip merging that data in for `__DEFAULT__`
segments, and repeat.

### How?
We currently assign `__DEFAULT__` to have `notFound()` behavior when
there isn't a `default.tsx` component for a particular segment. This
makes it so that when loading a page that renders a slot without slot
content / a `default`, it 404s. But when performing a client-side
navigation, the intended behavior is different: we keep whatever was in
the `default` slots place, until the user refreshes the page, which
would then 404.

However, this logic is incorrect when triggering any of the above
mentioned cache node revalidation strategies: if we always skip applying
to the `__DEFAULT__` segment, slots will never properly handle reducer
actions that rely on making changes to their cache nodes.

This splits these different `applyPatch` functions: one that will apply
to the full tree, and another that'll apply to everything except the
default segments with the existing bailout condition.

Fixes #54173
Fixes #58772
Fixes #54723
Fixes #57665

Closes NEXT-1706
Closes NEXT-1815
Closes NEXT-1812

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

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
+  )
+}

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
 }

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,

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,

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,

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,

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,

test/e2e/app-dir/parallel-routes-revalidation/app/@dialog/default.tsx

@@ -0,0 +1,3 @@
+export default function Default() {
+  return null
+}

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

@@ -0,0 +1,3 @@
+export default function Page() {
+  return null
+}

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>
+  )
+}

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>
+  )
+}

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>
+  )
+}

test/e2e/app-dir/parallel-routes-revalidation/app/@interception/default.tsx

@@ -0,0 +1,3 @@
+export default function Default() {
+  return null
+}

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('/')
+}

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>
+  )
+}

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

@@ -0,0 +1 @@
+export { default } from './page'