From 5ebe8b4d2f1521328299f60fe33cebbe7b57edaa Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 14 Jun 2023 13:54:12 +0200 Subject: [PATCH 1/3] invalidate client cache when cookie has changed --- .../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 | 68 +++++++++++++++++-- .../app-dir/actions/app/revalidate-2/page.js | 7 ++ .../app-dir/actions/app/revalidate/page.js | 2 +- 7 files changed, 126 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 c0a4c022fa541..7f045adae6e20 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 26b807a22de6a..cb9ba968c1a8d 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 ae5d5a3fce425..70b89b867fdc4 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 e9fdfc60482bf..d1824f0bd38ee 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 6c9d9532a5cd1..135b758bbea12 100644 --- a/test/e2e/app-dir/actions/app-action.test.ts +++ b/test/e2e/app-dir/actions/app-action.test.ts @@ -504,7 +504,7 @@ createNextDescribe( }) 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 +527,13 @@ 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 } // Should be different @@ -557,6 +560,61 @@ createNextDescribe( }, 'success') } ) + + it('should revalidate when cookies.set is called in a client action', async () => { + const browser = await next.browser('/revalidate') + + // Make sure we have a cookie set initially + await browser.elementByCss('#set-cookie').click() + + let randomCookie + await check(async () => { + randomCookie = JSON.parse( + await browser.elementByCss('#random-cookie').text() + ).value + return randomCookie ? 'success' : 'failure' + }, 'success') + + 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 + + // 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') + }) }) } ) 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 7e57bda9dc1fc..cd9641d3aca0f 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 a76f852973231..4cb957401d2dc 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

From 61758d054c66421d48e6d2b8d9de3efce1e57ae1 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 14 Jun 2023 16:25:03 +0200 Subject: [PATCH 2/3] adjust tests --- test/e2e/app-dir/actions/app-action.test.ts | 112 ++++++++++---------- 1 file changed, 57 insertions(+), 55 deletions(-) diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts index 135b758bbea12..94c8e89c75374 100644 --- a/test/e2e/app-dir/actions/app-action.test.ts +++ b/test/e2e/app-dir/actions/app-action.test.ts @@ -503,6 +503,63 @@ 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 revalidated', async (type) => { @@ -560,61 +617,6 @@ createNextDescribe( }, 'success') } ) - - it('should revalidate when cookies.set is called in a client action', async () => { - const browser = await next.browser('/revalidate') - - // Make sure we have a cookie set initially - await browser.elementByCss('#set-cookie').click() - - let randomCookie - await check(async () => { - randomCookie = JSON.parse( - await browser.elementByCss('#random-cookie').text() - ).value - return randomCookie ? 'success' : 'failure' - }, 'success') - - 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 - - // 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') - }) }) } ) From 6eb01396f9be09fa912ef04972e73dbc15d6c943 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Wed, 14 Jun 2023 16:31:24 +0200 Subject: [PATCH 3/3] fix lint --- test/e2e/app-dir/actions/app-action.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts index 94c8e89c75374..0e08360120606 100644 --- a/test/e2e/app-dir/actions/app-action.test.ts +++ b/test/e2e/app-dir/actions/app-action.test.ts @@ -591,6 +591,8 @@ createNextDescribe( case 'path': await browser.elementByCss('#revalidate-path').click() break + default: + throw new Error(`Invalid type: ${type}`) } // Should be different