Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalidate client cache when cookies have changed in Server Actions #51290

Merged
merged 4 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type FetchServerActionResult = {
actionFlightData?: FlightData | undefined | null
revalidatedParts: {
tag: boolean
cookie: boolean
paths: string[]
}
}
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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) {
Expand Down
43 changes: 36 additions & 7 deletions packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -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])
)
}

Expand Down Expand Up @@ -226,6 +244,7 @@ export async function handleAction({
serverActionsManifest,
generateFlight,
staticGenerationStore,
requestStore,
}: {
req: IncomingMessage
res: ServerResponse
Expand All @@ -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']
Expand Down Expand Up @@ -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),
Expand All @@ -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(
Expand All @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1617,6 +1624,7 @@ export async function renderToHTMLOrFlight(
serverActionsManifest,
generateFlight,
staticGenerationStore,
requestStore,
})

if (actionRequestResult === 'not-found') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
]
Expand Down
72 changes: 67 additions & 5 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/app-dir/actions/app/revalidate-2/page.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { revalidateTag } from 'next/cache'
import { cookies } from 'next/headers'
import Link from 'next/link'

export default async function Page() {
Expand Down Expand Up @@ -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>
</>
)
}
2 changes: 1 addition & 1 deletion test/e2e/app-dir/actions/app/revalidate/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down