From 754c6480cba7535ef95fb8a6beace53e07c5a601 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 14 Jun 2023 17:26:27 +0200 Subject: [PATCH] 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. --- .../reducers/server-action-reducer.ts | 13 ++-- .../src/server/app-render/action-handler.ts | 43 +++++++++-- .../next/src/server/app-render/app-render.tsx | 8 +++ .../adapters/request-cookies.ts | 4 +- test/e2e/app-dir/actions/app-action.test.ts | 72 +++++++++++++++++-- .../app-dir/actions/app/revalidate-2/page.js | 7 ++ .../app-dir/actions/app/revalidate/page.js | 2 +- 7 files changed, 130 insertions(+), 19 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 index c0a4c022fa..7f045adae6 100644 --- 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 @@ -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) { diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts index 26b807a22d..cb9ba968c1 100644 --- a/packages/next/src/server/app-render/action-handler.ts +++ b/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,7 +133,13 @@ function fetchIPv4v6( async function addRevalidationHeader( res: ServerResponse, - staticGenerationStore: StaticGenerationStore + { + staticGenerationStore, + requestStore, + }: { + staticGenerationStore: StaticGenerationStore + requestStore: RequestStore + } ) { await Promise.all(staticGenerationStore.pendingRevalidates || []) @@ -137,7 +147,8 @@ async function addRevalidationHeader( // 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, @@ -145,9 +156,16 @@ async function addRevalidationHeader( // 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 staticGenerationStore: StaticGenerationStore + requestStore: RequestStore }): Promise { 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) diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index ae5d5a3fce..70b89b867f 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/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') { diff --git a/packages/next/src/server/web/spec-extension/adapters/request-cookies.ts b/packages/next/src/server/web/spec-extension/adapters/request-cookies.ts index e9fdfc6048..d1824f0bd3 100644 --- a/packages/next/src/server/web/spec-extension/adapters/request-cookies.ts +++ b/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 ] diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts index 6c9d9532a5..0e08360120 100644 --- a/test/e2e/app-dir/actions/app-action.test.ts +++ b/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 diff --git a/test/e2e/app-dir/actions/app/revalidate-2/page.js b/test/e2e/app-dir/actions/app/revalidate-2/page.js index 7e57bda9dc..cd9641d3ac 100644 --- a/test/e2e/app-dir/actions/app/revalidate-2/page.js +++ b/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 +

+ random cookie:{' '} + + {JSON.stringify(cookies().get('random'))} + +

) } diff --git a/test/e2e/app-dir/actions/app/revalidate/page.js b/test/e2e/app-dir/actions/app/revalidate/page.js index a76f852973..4cb957401d 100644 --- a/test/e2e/app-dir/actions/app/revalidate/page.js +++ b/test/e2e/app-dir/actions/app/revalidate/page.js @@ -51,7 +51,7 @@ export default async function Page() { {' '} - /revalidate/another-route + /revalidate-2