actions: fill prefetchCache with revalidation payload (#49576)

This fixes a bug where the revalidation value was not stored into the prefetch cache, leading you to potentially see stale value when re-reading from the prefetch cache.  link NEXT-1134 Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>
vercel · May 11, 2023 · 02d020c · 02d020c
1 parent 1f561a6
commit 02d020c
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 7 deletions.
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
@@ -103,6 +103,7 @@ export function serverActionReducer(
 
   if (!action.mutable.inFlightServerAction) {
     action.mutable.previousTree = state.tree
+    action.mutable.previousUrl = state.canonicalUrl
     action.mutable.inFlightServerAction = createRecordFromThenable(
       fetchServerAction(state, action)
     )
@@ -117,11 +118,8 @@ export function serverActionReducer(
     if (redirectLocation) {
       // the redirection might have a flight data associated with it, so we'll populate the cache with it
       if (actionFlightData) {
-        const href = createHrefFromUrl(
-          redirectLocation,
-          // Ensures the hash is not part of the cache key as it does not affect fetching the server
-          false
-        )
+        const href = createHrefFromUrl(redirectLocation, false)
+        const previousCacheEntry = state.prefetchCache.get(href)
         state.prefetchCache.set(href, {
           data: createRecordFromThenable(
             Promise.resolve([
@@ -130,7 +128,7 @@ export function serverActionReducer(
               undefined,
             ])
           ),
-          kind: PrefetchKind.TEMPORARY, //TODO-APP: maybe this could cached longer?
+          kind: previousCacheEntry?.kind ?? PrefetchKind.TEMPORARY,
           prefetchTime: Date.now(),
           treeAtTimeOfPrefetch: action.mutable.previousTree!,
           lastUsedTime: null,
@@ -142,8 +140,31 @@ export function serverActionReducer(
         getRedirectError(redirectLocation.toString(), RedirectType.push)
       )
     } else {
-      // TODO-APP: populate the prefetch cache with the new flight data
       if (actionFlightData) {
+        const href = createHrefFromUrl(
+          new URL(action.mutable.previousUrl!, window.location.origin),
+          false
+        )
+        const previousCacheEntry = state.prefetchCache.get(href)
+        state.prefetchCache.set(
+          createHrefFromUrl(
+            new URL(action.mutable.previousUrl!, window.location.origin),
+            false
+          ),
+          {
+            data: createRecordFromThenable(
+              Promise.resolve([
+                actionFlightData,
+                // TODO-APP: verify the logic around canonical URL overrides
+                undefined,
+              ])
+            ),
+            kind: previousCacheEntry?.kind ?? PrefetchKind.TEMPORARY,
+            prefetchTime: Date.now(),
+            treeAtTimeOfPrefetch: action.mutable.previousTree!,
+            lastUsedTime: null,
+          }
+        )
         // this is an intentional hack around React: we want to update the tree in a new render
         setTimeout(() => {
           action.changeByServerResponse(

diff --git a/packages/next/src/client/components/router-reducer/router-reducer-types.ts b/packages/next/src/client/components/router-reducer/router-reducer-types.ts
@@ -42,6 +42,7 @@ export interface ServerActionMutable {
   inFlightServerAction?: Promise<any> | null
   serverActionApplied?: boolean
   previousTree?: FlightRouterState
+  previousUrl?: string
 }
 
 /**

diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts
@@ -431,6 +431,32 @@ createNextDescribe(
         }, 'success')
       })
 
+      it('should store revalidation data in the prefetch cache', async () => {
+        const browser = await next.browser('/client')
+        await browser.elementByCss('#navigate-revalidate').click()
+        const justPutIt = await browser.elementByCss('#justputit').text()
+        await browser.elementByCss('#revalidate-justputit').click()
+        await check(async () => {
+          const newJustPutIt = await browser.elementByCss('#justputit').text()
+          return newJustPutIt !== justPutIt ? 'success' : 'failure'
+        }, 'success')
+
+        const newJustPutIt = await browser.elementByCss('#justputit').text()
+
+        await browser
+          .elementByCss('#navigate-client')
+          .click()
+          .waitForElementByCss('#inc')
+        await browser
+          .elementByCss('#navigate-revalidate')
+          .click()
+          .waitForElementByCss('#revalidate-justputit')
+
+        const newJustPutIt2 = await browser.elementByCss('#justputit').text()
+
+        expect(newJustPutIt).toEqual(newJustPutIt2)
+      })
+
       it('should revalidate when cookies.set is called', async () => {
         const browser = await next.browser('/revalidate')
         const randomNumber = await browser.elementByCss('#random-cookie').text()

diff --git a/test/e2e/app-dir/actions/app/layout.js b/test/e2e/app-dir/actions/app/layout.js
@@ -1,9 +1,28 @@
+import Link from 'next/link'
+
 export default function RootLayout({ children }) {
   return (
     <html>
       <head />
       <body>
         <div id="random-number">{Math.random()}</div>
+        <div>
+          <div>
+            <Link id="navigate-client" href="/client">
+              Client
+            </Link>
+          </div>
+          <div>
+            <Link id="navigate-server" href="/server">
+              Server
+            </Link>
+          </div>
+          <div>
+            <Link id="navigate-revalidate" href="/revalidate">
+              Client and Server
+            </Link>
+          </div>
+        </div>
         {children}
       </body>
     </html>