Skip to content

Commit

Permalink
fix: set netlify-cache-tag for not prerendered content (#2495)
Browse files Browse the repository at this point in the history
* test: refactor app router on-demand revalidation test and add test cases for not prerendered content

* test: refactor pages router on-demand revalidation test and add test cases for not prerendered content

* fix: capture cache tags during request handling and don't rely on tag manifest created for prerendered pages

* Update src/run/handlers/cache.cts

Co-authored-by: Philippe Serhal <[email protected]>

* chore: remove dead code

---------

Co-authored-by: Philippe Serhal <[email protected]>
  • Loading branch information
pieh and serhalp authored Jun 24, 2024
1 parent cfe8c59 commit 8fe6676
Show file tree
Hide file tree
Showing 19 changed files with 1,023 additions and 1,087 deletions.
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ For a simple next.js app
```
/___netlify-server-handler
├── .netlify
│ ├── tags-manifest.json
│ └── dist // the compiled runtime code
│ └── run
│ ├── handlers
Expand Down
41 changes: 0 additions & 41 deletions src/build/content/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,47 +311,6 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> =>
})
}

export const writeTagsManifest = async (ctx: PluginContext): Promise<void> => {
const manifest = await ctx.getPrerenderManifest()

const routes = Object.entries(manifest.routes).map(async ([route, definition]) => {
let tags

// app router
if (definition.dataRoute?.endsWith('.rsc')) {
const path = join(ctx.publishDir, `server/app/${route === '/' ? '/index' : route}.meta`)
try {
const file = await readFile(path, 'utf-8')
const meta = JSON.parse(file)
tags = meta.headers['x-next-cache-tags']
} catch {
// Parallel route default layout has no prerendered page, so don't warn about it
if (!definition.dataRoute?.endsWith('/default.rsc')) {
console.log(`Unable to read cache tags for: ${path}`)
}
}
}

// pages router
if (definition.dataRoute?.endsWith('.json')) {
tags = `_N_T_${route}`
}

// route handler
if (definition.dataRoute === null) {
tags = definition.initialHeaders?.['x-next-cache-tags']
}

return [route, tags]
})

await writeFile(
join(ctx.serverHandlerDir, '.netlify/tags-manifest.json'),
JSON.stringify(Object.fromEntries(await Promise.all(routes))),
'utf-8',
)
}

/**
* Generates a copy of the middleware manifest without any middleware in it. We
* do this because we'll run middleware in an edge function, and we don't want
Expand Down
2 changes: 0 additions & 2 deletions src/build/functions/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
copyNextDependencies,
copyNextServerCode,
verifyHandlerDirStructure,
writeTagsManifest,
} from '../content/server.js'
import { PluginContext, SERVER_HANDLER_NAME } from '../plugin-context.js'

Expand Down Expand Up @@ -138,7 +137,6 @@ export const createServerHandler = async (ctx: PluginContext) => {

await copyNextServerCode(ctx)
await copyNextDependencies(ctx)
await writeTagsManifest(ctx)
await copyHandlerDependencies(ctx)
await writeHandlerManifest(ctx)
await writePackageMetadata(ctx)
Expand Down
6 changes: 0 additions & 6 deletions src/run/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,3 @@ export const setRunConfig = (config: NextConfigComplete) => {
// set config
process.env.__NEXT_PRIVATE_STANDALONE_CONFIG = JSON.stringify(config)
}

export type TagsManifest = Record<string, string>

export const getTagsManifest = async (): Promise<TagsManifest> => {
return JSON.parse(await readFile(resolve(PLUGIN_DIR, '.netlify/tags-manifest.json'), 'utf-8'))
}
43 changes: 43 additions & 0 deletions src/run/handlers/cache.cts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,44 @@ export class NetlifyCacheHandler implements CacheHandler {
return restOfRouteValue
}

private captureCacheTags(cacheValue: NetlifyIncrementalCacheValue | null, key: string) {
if (!cacheValue) {
return
}

const requestContext = getRequestContext()
// Bail if we can't get request context
if (!requestContext) {
return
}

// Bail if we already have cache tags - `captureCacheTags()` is called on both `CacheHandler.get` and `CacheHandler.set`
// that's because `CacheHandler.get` might not have a cache value (cache miss or on-demand revalidation) in which case
// response is generated in blocking way and we need to capture cache tags from the cache value we are setting.
// If both `CacheHandler.get` and `CacheHandler.set` are called in the same request, we want to use cache tags from
// first `CacheHandler.get` and not from following `CacheHandler.set` as this is pattern for Stale-while-revalidate behavior
// and stale response is served while new one is generated.
if (requestContext.responseCacheTags) {
return
}

if (
cacheValue.kind === 'PAGE' ||
cacheValue.kind === 'APP_PAGE' ||
cacheValue.kind === 'ROUTE'
) {
if (cacheValue.headers?.[NEXT_CACHE_TAGS_HEADER]) {
const cacheTags = (cacheValue.headers[NEXT_CACHE_TAGS_HEADER] as string).split(',')
requestContext.responseCacheTags = cacheTags
} else if (cacheValue.kind === 'PAGE' && typeof cacheValue.pageData === 'object') {
// pages router doesn't have cache tags headers in PAGE cache value
// so we need to generate appropriate cache tags for it
const cacheTags = [`_N_T_${key === '/index' ? '/' : key}`]
requestContext.responseCacheTags = cacheTags
}
}
}

private async injectEntryToPrerenderManifest(
key: string,
revalidate: NetlifyCachedPageValue['revalidate'],
Expand Down Expand Up @@ -176,6 +214,7 @@ export class NetlifyCacheHandler implements CacheHandler {
}

this.captureResponseCacheLastModified(blob, key, span)
this.captureCacheTags(blob.value, key)

switch (blob.value?.kind) {
case 'FETCH':
Expand Down Expand Up @@ -273,6 +312,10 @@ export class NetlifyCacheHandler implements CacheHandler {

const value = this.transformToStorableObject(data, context)

// if previous CacheHandler.get call returned null (page was either never rendered or was on-demand revalidated)
// and we didn't yet capture cache tags, we try to get cache tags from freshly produced cache value
this.captureCacheTags(value, key)

await this.blobStore.setJSON(blobKey, {
lastModified,
value,
Expand Down
1 change: 1 addition & 0 deletions src/run/handlers/request-context.cts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export type RequestContext = {
captureServerTiming: boolean
responseCacheGetLastModified?: number
responseCacheKey?: string
responseCacheTags?: string[]
usedFsRead?: boolean
didPagesRouterOnDemandRevalidate?: boolean
serverTiming?: string
Expand Down
17 changes: 3 additions & 14 deletions src/run/handlers/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { Context } from '@netlify/functions'
import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
import type { WorkerRequestHandler } from 'next/dist/server/lib/types.js'

import { getTagsManifest, TagsManifest } from '../config.js'
import {
adjustDateHeader,
setCacheControlHeaders,
Expand All @@ -20,7 +19,7 @@ import { getTracer } from './tracer.cjs'

const nextImportPromise = import('../next.cjs')

let nextHandler: WorkerRequestHandler, nextConfig: NextConfigComplete, tagsManifest: TagsManifest
let nextHandler: WorkerRequestHandler, nextConfig: NextConfigComplete

/**
* When Next.js proxies requests externally, it writes the response back as-is.
Expand Down Expand Up @@ -55,21 +54,11 @@ export default async (request: Request, context: FutureContext) => {
const tracer = getTracer()

if (!nextHandler) {
await tracer.withActiveSpan('initialize next server', async (span) => {
await tracer.withActiveSpan('initialize next server', async () => {
// set the server config
const { getRunConfig, setRunConfig } = await import('../config.js')
nextConfig = await getRunConfig()
setRunConfig(nextConfig)
tagsManifest = await getTagsManifest()
span.setAttributes(
Object.entries(tagsManifest).reduce(
(acc, [key, value]) => {
acc[`tagsManifest.${key}`] = value
return acc
},
{} as Record<string, string>,
),
)

const { getMockedRequestHandlers } = await nextImportPromise
const url = new URL(request.url)
Expand Down Expand Up @@ -124,7 +113,7 @@ export default async (request: Request, context: FutureContext) => {
await adjustDateHeader({ headers: response.headers, request, span, tracer, requestContext })

setCacheControlHeaders(response.headers, request, requestContext)
setCacheTagsHeaders(response.headers, request, tagsManifest, requestContext)
setCacheTagsHeaders(response.headers, requestContext)
setVaryHeaders(response.headers, request, nextConfig)
setCacheStatusHeader(response.headers)

Expand Down
19 changes: 3 additions & 16 deletions src/run/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { NextConfigComplete } from 'next/dist/server/config-shared.js'

import { encodeBlobKey } from '../shared/blobkey.js'

import type { TagsManifest } from './config.js'
import type { RequestContext } from './handlers/request-context.cjs'
import type { RuntimeTracer } from './handlers/tracer.cjs'
import { getRegionalBlobStore } from './regional-blob-store.cjs'
Expand Down Expand Up @@ -275,21 +274,9 @@ export const setCacheControlHeaders = (
}
}

function getCanonicalPathFromCacheKey(cacheKey: string | undefined): string | undefined {
return cacheKey === '/index' ? '/' : cacheKey
}

export const setCacheTagsHeaders = (
headers: Headers,
request: Request,
manifest: TagsManifest,
requestContext: RequestContext,
) => {
const path =
getCanonicalPathFromCacheKey(requestContext.responseCacheKey) ?? new URL(request.url).pathname
const tags = manifest[path]
if (tags !== undefined) {
headers.set('netlify-cache-tag', tags)
export const setCacheTagsHeaders = (headers: Headers, requestContext: RequestContext) => {
if (requestContext.responseCacheTags) {
headers.set('netlify-cache-tag', requestContext.responseCacheTags.join(','))
}
}

Expand Down
Loading

0 comments on commit 8fe6676

Please sign in to comment.