Invalidate client cache when cookies have changed in Server Actions (#…

…51290)

Similar to tags and paths, when `cookies().set()` is called we have to
invalidate the client router cache as well so routes with
`cookies().get()` will not holding the stale data. This is critical for
auth and other scenarios.

In the future we can have an optimization to only invalidate routes that
actually rely on cookies, similar to paths.

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

@@ -32,6 +32,7 @@ type FetchServerActionResult = {
   actionFlightData?: FlightData | undefined | null
   revalidatedParts: {
     tag: boolean
+    cookie: boolean
     paths: string[]
   }
 }
@@ -67,16 +68,18 @@ async function fetchServerAction(
   let revalidatedParts: FetchServerActionResult['revalidatedParts']
   try {
     const revalidatedHeader = JSON.parse(
-      res.headers.get('x-action-revalidated') || '[0,[]]'
+      res.headers.get('x-action-revalidated') || '[[],0,0]'
     )
     revalidatedParts = {
-      tag: !!revalidatedHeader[0],
-      paths: revalidatedHeader[1] || [],
+      paths: revalidatedHeader[0] || [],
+      tag: !!revalidatedHeader[1],
+      cookie: revalidatedHeader[2],
     }
   } catch (e) {
     revalidatedParts = {
-      tag: false,
       paths: [],
+      tag: false,
+      cookie: false,
     }
   }
 
@@ -153,7 +156,7 @@ export function serverActionReducer(
 
     // Invalidate the cache for the revalidated parts. This has to be done before the
     // cache is updated with the action's flight data again.
-    if (revalidatedParts.tag) {
+    if (revalidatedParts.tag || revalidatedParts.cookie) {
       // Invalidate everything if the tag is set.
       state.prefetchCache.clear()
     } else if (revalidatedParts.paths.length > 0) {

packages/next/src/server/app-render/action-handler.ts

@@ -22,7 +22,11 @@ import { FlightRenderResult } from './flight-render-result'
 import { ActionResult } from './types'
 import { ActionAsyncStorage } from '../../client/components/action-async-storage'
 import { filterReqHeaders, forbiddenHeaders } from '../lib/server-ipc/utils'
-import { appendMutableCookies } from '../web/spec-extension/adapters/request-cookies'
+import {
+  appendMutableCookies,
+  getModifiedCookieValues,
+} from '../web/spec-extension/adapters/request-cookies'
+import { RequestStore } from '../../client/components/request-async-storage'
 
 function nodeToWebReadableStream(nodeReadable: import('stream').Readable) {
   if (process.env.NEXT_RUNTIME !== 'edge') {
@@ -129,25 +133,39 @@ function fetchIPv4v6(
 
 async function addRevalidationHeader(
   res: ServerResponse,
-  staticGenerationStore: StaticGenerationStore
+  {
+    staticGenerationStore,
+    requestStore,
+  }: {
+    staticGenerationStore: StaticGenerationStore
+    requestStore: RequestStore
+  }
 ) {
   await Promise.all(staticGenerationStore.pendingRevalidates || [])
 
   // If a tag was revalidated, the client router needs to invalidate all the
   // client router cache as they may be stale. And if a path was revalidated, the
   // client needs to invalidate all subtrees below that path.
 
-  // To keep the header size small, we use a tuple of [isTagRevalidated ? 1 : 0, [paths]]
+  // To keep the header size small, we use a tuple of
+  // [[revalidatedPaths], isTagRevalidated ? 1 : 0, isCookieRevalidated ? 1 : 0]
   // instead of a JSON object.
 
   // TODO-APP: Currently the prefetch cache doesn't have subtree information,
   // so we need to invalidate the entire cache if a path was revalidated.
   // TODO-APP: Currently paths are treated as tags, so the second element of the tuple
   // is always empty.
 
+  const isTagRevalidated = staticGenerationStore.revalidatedTags?.length ? 1 : 0
+  const isCookieRevalidated = getModifiedCookieValues(
+    requestStore.mutableCookies
+  ).length
+    ? 1
+    : 0
+
   res.setHeader(
     'x-action-revalidated',
-    JSON.stringify([staticGenerationStore.revalidatedTags?.length ? 1 : 0, []])
+    JSON.stringify([[], isTagRevalidated, isCookieRevalidated])
   )
 }
 
@@ -226,6 +244,7 @@ export async function handleAction({
   serverActionsManifest,
   generateFlight,
   staticGenerationStore,
+  requestStore,
 }: {
   req: IncomingMessage
   res: ServerResponse
@@ -238,6 +257,7 @@ export async function handleAction({
     asNotFound?: boolean
   }) => Promise<RenderResult>
   staticGenerationStore: StaticGenerationStore
+  requestStore: RequestStore
 }): Promise<undefined | RenderResult | 'not-found'> {
   let actionId = req.headers[ACTION.toLowerCase()] as string
   const contentType = req.headers['content-type']
@@ -374,7 +394,10 @@ export async function handleAction({
 
         // For form actions, we need to continue rendering the page.
         if (isFetchAction) {
-          await addRevalidationHeader(res, staticGenerationStore)
+          await addRevalidationHeader(res, {
+            staticGenerationStore,
+            requestStore,
+          })
 
           actionResult = await generateFlight({
             actionResult: Promise.resolve(returnVal),
@@ -391,7 +414,10 @@ export async function handleAction({
 
         // if it's a fetch action, we don't want to mess with the status code
         // and we'll handle it on the client router
-        await addRevalidationHeader(res, staticGenerationStore)
+        await addRevalidationHeader(res, {
+          staticGenerationStore,
+          requestStore,
+        })
 
         if (isFetchAction) {
           return createRedirectRenderResult(
@@ -418,7 +444,10 @@ export async function handleAction({
       } else if (isNotFoundError(err)) {
         res.statusCode = 404
 
-        await addRevalidationHeader(res, staticGenerationStore)
+        await addRevalidationHeader(res, {
+          staticGenerationStore,
+          requestStore,
+        })
 
         if (isFetchAction) {
           const promise = Promise.reject(err)

packages/next/src/server/app-render/app-render.tsx

@@ -224,6 +224,13 @@ export async function renderToHTMLOrFlight(
     staticGenerationStore.fetchMetrics = []
     ;(renderOpts as any).fetchMetrics = staticGenerationStore.fetchMetrics
 
+    const requestStore = requestAsyncStorage.getStore()
+    if (!requestStore) {
+      throw new Error(
+        `Invariant: Render expects to have requestAsyncStorage, none found`
+      )
+    }
+
     // don't modify original query object
     query = { ...query }
     stripInternalQueries(query)
@@ -1617,6 +1624,7 @@ export async function renderToHTMLOrFlight(
       serverActionsManifest,
       generateFlight,
       staticGenerationStore,
+      requestStore,
     })
 
     if (actionRequestResult === 'not-found') {

packages/next/src/server/web/spec-extension/adapters/request-cookies.ts

@@ -49,7 +49,9 @@ export class RequestCookiesAdapter {
 
 const SYMBOL_MODIFY_COOKIE_VALUES = Symbol.for('next.mutated.cookies')
 
-function getModifiedCookieValues(cookies: ResponseCookies): ResponseCookie[] {
+export function getModifiedCookieValues(
+  cookies: ResponseCookies
+): ResponseCookie[] {
   const modified: ResponseCookie[] | undefined = (cookies as unknown as any)[
     SYMBOL_MODIFY_COOKIE_VALUES
   ]

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

@@ -503,8 +503,65 @@ createNextDescribe(
         }, 'success')
       })
 
+      it('should revalidate when cookies.set is called in a client action', async () => {
+        const browser = await next.browser('/revalidate')
+        await browser.refresh()
+
+        let randomCookie
+        await check(async () => {
+          randomCookie = JSON.parse(
+            await browser.elementByCss('#random-cookie').text()
+          ).value
+          return randomCookie ? 'success' : 'failure'
+        }, 'success')
+
+        console.log(123, await browser.elementByCss('body').text())
+
+        await browser.elementByCss('#another').click()
+        await check(async () => {
+          return browser.elementByCss('#title').text()
+        }, 'another route')
+
+        const newRandomCookie = JSON.parse(
+          await browser.elementByCss('#random-cookie').text()
+        ).value
+
+        console.log(456, await browser.elementByCss('body').text())
+
+        // Should be the same value
+        expect(randomCookie).toEqual(newRandomCookie)
+
+        await browser.elementByCss('#back').click()
+
+        // Modify the cookie
+        await browser.elementByCss('#set-cookie').click()
+
+        // Should be different
+        let revalidatedRandomCookie
+        await check(async () => {
+          revalidatedRandomCookie = JSON.parse(
+            await browser.elementByCss('#random-cookie').text()
+          ).value
+          return randomCookie !== revalidatedRandomCookie
+            ? 'success'
+            : 'failure'
+        }, 'success')
+
+        await browser.elementByCss('#another').click()
+
+        // The other page should be revalidated too
+        await check(async () => {
+          const newRandomCookie = await JSON.parse(
+            await browser.elementByCss('#random-cookie').text()
+          ).value
+          return revalidatedRandomCookie === newRandomCookie
+            ? 'success'
+            : 'failure'
+        }, 'success')
+      })
+
       it.each(['tag', 'path'])(
-        'should invalidate client cache when %s is revalidate',
+        'should invalidate client cache when %s is revalidated',
         async (type) => {
           const browser = await next.browser('/revalidate')
           await browser.refresh()
@@ -527,10 +584,15 @@ createNextDescribe(
 
           await browser.elementByCss('#back').click()
 
-          if (type === 'tag') {
-            await browser.elementByCss('#revalidate-thankyounext').click()
-          } else {
-            await browser.elementByCss('#revalidate-path').click()
+          switch (type) {
+            case 'tag':
+              await browser.elementByCss('#revalidate-thankyounext').click()
+              break
+            case 'path':
+              await browser.elementByCss('#revalidate-path').click()
+              break
+            default:
+              throw new Error(`Invalid type: ${type}`)
           }
 
           // Should be different

test/e2e/app-dir/actions/app/revalidate-2/page.js

@@ -1,4 +1,5 @@
 import { revalidateTag } from 'next/cache'
+import { cookies } from 'next/headers'
 import Link from 'next/link'
 
 export default async function Page() {
@@ -30,6 +31,12 @@ export default async function Page() {
           revalidate thankyounext
         </button>
       </form>
+      <p>
+        random cookie:{' '}
+        <span id="random-cookie">
+          {JSON.stringify(cookies().get('random'))}
+        </span>
+      </p>
     </>
   )
 }

test/e2e/app-dir/actions/app/revalidate/page.js

@@ -51,7 +51,7 @@ export default async function Page() {
         </span>{' '}
         <span>
           <Link href="/revalidate-2" id="another">
-            /revalidate/another-route
+            /revalidate-2
           </Link>
         </span>
       </p>