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

fix: set netlify-cache-tag for not prerendered content #2495

Merged
merged 6 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can delete this too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... Oh, I don't think this is related to your changes, but still dead code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not really related to this change, but I did remove it now


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
}
Comment on lines +123 to +131
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to think of something less leaky but I can't 😞


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 on was on-demand revalidated)
pieh marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading