client router cache should still be cleared if server action revalida…

…tes + redirects (#70715)

If a server action redirects, we reject the promise eagerly with a
redirect error, but that means we skip the handling that clears the
client router cache in case the response was revalidated.

This keeps the existing handling to seed the prefetch cache & reject the
promise but ensures we've had a chance to apply the flight data to the
router state.

Fixes #70483
Closes NDX-357

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

@@ -216,6 +216,7 @@ export function serverActionReducer(
       redirectLocation,
       redirectType,
       isPrerender,
+      revalidatedParts,
     }) => {
       // honor the redirect type instead of defaulting to push in case of server actions.
       if (redirectLocation) {
@@ -253,44 +254,10 @@ export function serverActionReducer(
         )
       }
 
-      if (redirectLocation) {
-        const newHref = createHrefFromUrl(redirectLocation, false)
-
-        // Because the RedirectBoundary will trigger a navigation, we need to seed the prefetch cache
-        // with the FlightData that we got from the server action for the target page, so that it's
-        // available when the page is navigated to and doesn't need to be re-fetched.
-        createSeededPrefetchCacheEntry({
-          url: redirectLocation,
-          data: {
-            flightData,
-            canonicalUrl: undefined,
-            couldBeIntercepted: false,
-            prerendered: false,
-            postponed: false,
-          },
-          tree: state.tree,
-          prefetchCache: state.prefetchCache,
-          nextUrl: state.nextUrl,
-          kind: isPrerender ? PrefetchKind.FULL : PrefetchKind.AUTO,
-        })
-
-        mutable.prefetchCache = state.prefetchCache
-        // If the action triggered a redirect, we reject the action promise with a redirect
-        // so that it's handled by RedirectBoundary as we won't have a valid
-        // action result to resolve the promise with. This will effectively reset the state of
-        // the component that called the action as the error boundary will remount the tree.
-        // The status code doesn't matter here as the action handler will have already sent
-        // a response with the correct status code.
-
-        reject(
-          getRedirectError(
-            hasBasePath(newHref) ? removeBasePath(newHref) : newHref,
-            redirectType || RedirectType.push
-          )
-        )
-
-        return handleMutable(state, mutable)
-      }
+      const actionRevalidated =
+        revalidatedParts.paths.length > 0 ||
+        revalidatedParts.tag ||
+        revalidatedParts.cookie
 
       for (const normalizedFlightData of flightData) {
         const {
@@ -330,7 +297,7 @@ export function serverActionReducer(
           )
         }
 
-        // Handles case where prefetch only returns the router tree patch without rendered components.
+        // The server sent back RSC data for the server action, so we need to apply it to the cache.
         if (cacheNodeSeedData !== null) {
           const rsc = cacheNodeSeedData[1]
           const cache: CacheNode = createEmptyCacheNode()
@@ -339,30 +306,73 @@ export function serverActionReducer(
           cache.loading = cacheNodeSeedData[3]
           fillLazyItemsTillLeafWithHead(
             cache,
-            // Existing cache is not passed in as `router.refresh()` has to invalidate the entire cache.
+            // Existing cache is not passed in as server actions have to invalidate the entire cache.
             undefined,
             treePatch,
             cacheNodeSeedData,
             head
           )
 
-          await refreshInactiveParallelSegments({
-            state,
-            updatedTree: newTree,
-            updatedCache: cache,
-            includeNextUrl: Boolean(nextUrl),
-            canonicalUrl: mutable.canonicalUrl || state.canonicalUrl,
-          })
-
           mutable.cache = cache
           mutable.prefetchCache = new Map()
+
+          if (actionRevalidated) {
+            await refreshInactiveParallelSegments({
+              state,
+              updatedTree: newTree,
+              updatedCache: cache,
+              includeNextUrl: Boolean(nextUrl),
+              canonicalUrl: mutable.canonicalUrl || state.canonicalUrl,
+            })
+          }
         }
 
         mutable.patchedTree = newTree
         currentTree = newTree
       }
 
-      resolve(actionResult)
+      if (redirectLocation) {
+        const newHref = createHrefFromUrl(redirectLocation, false)
+        mutable.canonicalUrl = newHref
+
+        // Because the RedirectBoundary will trigger a navigation, we need to seed the prefetch cache
+        // with the FlightData that we got from the server action for the target page, so that it's
+        // available when the page is navigated to and doesn't need to be re-fetched.
+        // We only do this if the server action didn't revalidate any data, as in that case the
+        // client cache will be cleared and the data will be re-fetched anyway.
+        if (!actionRevalidated) {
+          createSeededPrefetchCacheEntry({
+            url: redirectLocation,
+            data: {
+              flightData,
+              canonicalUrl: undefined,
+              couldBeIntercepted: false,
+              prerendered: false,
+              postponed: false,
+            },
+            tree: state.tree,
+            prefetchCache: state.prefetchCache,
+            nextUrl: state.nextUrl,
+            kind: isPrerender ? PrefetchKind.FULL : PrefetchKind.AUTO,
+          })
+          mutable.prefetchCache = state.prefetchCache
+        }
+
+        // If the action triggered a redirect, the action promise promise will be rejected with
+        // a redirect so that it's handled by RedirectBoundary as we won't have a valid
+        // action result to resolve the promise with. This will effectively reset the state of
+        // the component that called the action as the error boundary will remount the tree.
+        // The status code doesn't matter here as the action handler will have already sent
+        // a response with the correct status code.
+        reject(
+          getRedirectError(
+            hasBasePath(newHref) ? removeBasePath(newHref) : newHref,
+            redirectType || RedirectType.push
+          )
+        )
+      } else {
+        resolve(actionResult)
+      }
 
       return handleMutable(state, mutable)
     },

test/e2e/app-dir/actions/app-action.test.ts

@@ -659,8 +659,49 @@ describe('app-dir action handling', () => {
     })
   })
 
+  it('should invalidate the client router cache if the redirect action triggers a revalidation', async () => {
+    const browser = await next.browser('/redirect')
+    const input = await browser.elementByCss('input[name="name"]')
+    const revalidateCheckbox = await browser.elementByCss(
+      'input[name="revalidate"]'
+    )
+    const submit = await browser.elementByCss('button')
+    const initialRandom = await browser.elementById('random-number').text()
+    expect(initialRandom).toMatch(/\d+/)
+
+    expect(await browser.hasElementByCssSelector('#error')).toBe(false)
+
+    await input.fill('justputit')
+    await revalidateCheckbox.check()
+    await submit.click()
+
+    await retry(async () => {
+      expect(await browser.hasElementByCssSelector('#error')).toBe(false)
+    })
+
+    // go back to the page that was revalidated
+    await browser.elementByCss('[href="/redirect"]').click()
+
+    await browser.waitForElementByCss('#main-page')
+
+    const newRandom = await browser.elementById('random-number').text()
+    expect(newRandom).not.toBe(initialRandom)
+  })
+
   it('should reset the form state when the action redirects to itself', async () => {
     const browser = await next.browser('/self-redirect')
+    const requests = []
+    browser.on('request', async (req: Request) => {
+      const url = new URL(req.url())
+
+      if (url.pathname === '/self-redirect') {
+        const headers = await req.allHeaders()
+        if (headers['rsc']) {
+          requests.push(req)
+        }
+      }
+    })
+
     const input = await browser.elementByCss('input[name="name"]')
     const submit = await browser.elementByCss('button')
 
@@ -686,6 +727,13 @@ describe('app-dir action handling', () => {
     await retry(async () => {
       expect(await browser.hasElementByCssSelector('#error')).toBe(false)
     })
+
+    // This verifies the redirect & server response happens in a single roundtrip,
+    // if the redirect resource was static. In development, these responses are always
+    // dynamically generated, so we only expect a single request for build/deploy.
+    if (!isNextDev) {
+      expect(requests.length).toBe(0)
+    }
   })
 
   // This is disabled when deployed because the 404 page will be served as a static route

test/e2e/app-dir/actions/app/redirect/actions.ts

@@ -1,17 +1,23 @@
 'use server'
 
 import { redirect } from 'next/navigation'
+import { revalidatePath } from 'next/cache'
 
 type State = {
   errors: Record<string, string>
 }
 
 export async function action(previousState: State, formData: FormData) {
   const name = formData.get('name')
+  const revalidate = formData.get('revalidate')
 
   if (name !== 'justputit') {
     return { errors: { name: "Only 'justputit' is accepted." } }
   }
 
+  if (revalidate === 'on') {
+    revalidatePath('/redirect')
+  }
+
   redirect('/redirect/other')
 }

test/e2e/app-dir/actions/app/redirect/layout.tsx

@@ -12,6 +12,7 @@ export default function Page({ children }) {
     <div>
       <form action={dispatch}>
         <input type="text" name="name" />
+        <input type="checkbox" name="revalidate" /> Revalidate
         <button type="submit">Submit</button>
         {errors.name && <p id="error">{errors.name}</p>}
       </form>

test/e2e/app-dir/actions/app/redirect/other/page.tsx

@@ -1,3 +1,9 @@
+import Link from 'next/link'
+
 export default function Page() {
-  return <div>Other Page</div>
+  return (
+    <div>
+      Other Page <Link href="/redirect">Back to Redirect Page</Link>
+    </div>
+  )
 }

test/e2e/app-dir/actions/app/redirect/page.tsx

@@ -1,3 +1,7 @@
 export default function Page() {
-  return <div>Main Page</div>
+  return (
+    <div id="main-page">
+      Main Page <div id="rand">{Math.random()}</div>
+    </div>
+  )
 }