fix: cache freshness for ISR content (#235)

* Revert mtime fix and amend cache entry freshness

fix: revert commit 4e5d5fc

fix: exclude fetch responses from build cache

fix: stop uploading fetch cache entries to blob store

fix: upload prerendered content to blob store as stale

fix: only upload content as stale for revalidate

* chore: use next types for cache values

* fix: use a truthy lastModified value in the past

* test: update tests

* chore: remove only on test

* feat: reinstate fetch cache but stale

* chore: fix types to use Next.js types

* fix: delete fetch-cache files before saving to cache

* fix: add cache restore to utils mock

Author: orinokai

src/build/cache.ts

@@ -1,24 +1,33 @@
 import { existsSync } from 'node:fs'
+import { rm } from 'node:fs/promises'
 import { join } from 'node:path'
 
 import type { PluginContext } from './plugin-context.js'
 
 export const saveBuildCache = async (ctx: PluginContext) => {
-  if (await ctx.utils.cache.save(join(ctx.publishDir, 'cache'))) {
-    console.log('Next.js cache saved.')
+  const { cache } = ctx.utils
+  const cacheDir = join(ctx.publishDir, 'cache')
+
+  if (existsSync(cacheDir)) {
+    // remove the fetch responses because they are never updated once
+    // created at build time and would always be stale if saved
+    await rm(join(cacheDir, 'fetch-cache'), { recursive: true, force: true })
+
+    await cache.save(cacheDir)
+
+    console.log('Next.js cache saved')
   } else {
-    console.log('No Next.js cache to save.')
+    console.log('No Next.js cache to save')
   }
 }
 
 export const restoreBuildCache = async (ctx: PluginContext) => {
   const { cache } = ctx.utils
+  const cacheDir = join(ctx.publishDir, 'cache')
 
-  if (existsSync(join(ctx.publishDir, 'cache'))) {
-    console.log('Next.js cache found.')
-  } else if (await cache.restore(join(ctx.publishDir, 'cache'))) {
-    console.log('Next.js cache restored.')
+  if (await cache.restore(cacheDir)) {
+    console.log('Next.js cache restored')
   } else {
-    console.log('No Next.js cache to restore.')
+    console.log('No Next.js cache to restore')
   }
 }

src/build/content/prerendered.ts

@@ -1,42 +1,65 @@
 import { existsSync } from 'node:fs'
-import { readFile } from 'node:fs/promises'
-import { join } from 'node:path'
+import { mkdir, readFile, writeFile } from 'node:fs/promises'
+import { dirname, join } from 'node:path'
 
-import glob from 'fast-glob'
+import { glob } from 'fast-glob'
+import type { CacheHandlerValue } from 'next/dist/server/lib/incremental-cache/index.js'
+import type { IncrementalCacheValue } from 'next/dist/server/response-cache/types.js'
 
-import type {
-  CacheValue,
-  FetchCacheValue,
-  PageCacheValue,
-  PluginContext,
-  RouteCacheValue,
-} from '../plugin-context.js'
+import { encodeBlobKey } from '../../shared/blobkey.js'
+import type { PluginContext } from '../plugin-context.js'
+
+type CachedPageValue = Extract<IncrementalCacheValue, { kind: 'PAGE' }>
+type CachedRouteValue = Extract<IncrementalCacheValue, { kind: 'ROUTE' }>
+type CachedFetchValue = Extract<IncrementalCacheValue, { kind: 'FETCH' }>
+
+/**
+ * Write a cache entry to the blob upload directory.
+ */
+const writeCacheEntry = async (
+  route: string,
+  value: IncrementalCacheValue,
+  lastModified: number,
+  ctx: PluginContext,
+): Promise<void> => {
+  const path = join(ctx.blobDir, await encodeBlobKey(route))
+  const entry = JSON.stringify({
+    lastModified,
+    value,
+  } satisfies CacheHandlerValue)
+
+  await mkdir(dirname(path), { recursive: true })
+  await writeFile(path, entry, 'utf-8')
+}
 
 /**
  * Normalize routes by stripping leading slashes and ensuring root path is index
  */
 const routeToFilePath = (path: string) => (path === '/' ? '/index' : path)
 
-const buildPagesCacheValue = async (path: string): Promise<PageCacheValue> => ({
+const buildPagesCacheValue = async (path: string): Promise<CachedPageValue> => ({
   kind: 'PAGE',
   html: await readFile(`${path}.html`, 'utf-8'),
   pageData: JSON.parse(await readFile(`${path}.json`, 'utf-8')),
+  postponed: undefined,
+  headers: undefined,
+  status: undefined,
 })
 
-const buildAppCacheValue = async (path: string): Promise<PageCacheValue> => ({
+const buildAppCacheValue = async (path: string): Promise<CachedPageValue> => ({
   kind: 'PAGE',
   html: await readFile(`${path}.html`, 'utf-8'),
   pageData: await readFile(`${path}.rsc`, 'utf-8'),
   ...JSON.parse(await readFile(`${path}.meta`, 'utf-8')),
 })
 
-const buildRouteCacheValue = async (path: string): Promise<RouteCacheValue> => ({
+const buildRouteCacheValue = async (path: string): Promise<CachedRouteValue> => ({
   kind: 'ROUTE',
   body: await readFile(`${path}.body`, 'base64'),
   ...JSON.parse(await readFile(`${path}.meta`, 'utf-8')),
 })
 
-const buildFetchCacheValue = async (path: string): Promise<FetchCacheValue> => ({
+const buildFetchCacheValue = async (path: string): Promise<CachedFetchValue> => ({
   kind: 'FETCH',
   ...JSON.parse(await readFile(path, 'utf-8')),
 })
@@ -51,45 +74,38 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
 
     await Promise.all(
       Object.entries(manifest.routes).map(async ([route, meta]): Promise<void> => {
+        const lastModified = meta.initialRevalidateSeconds ? 1 : Date.now()
         const key = routeToFilePath(route)
-        let value: CacheValue
-        let path: string
+        let value: IncrementalCacheValue
         switch (true) {
           // Parallel route default layout has no prerendered page
           case meta.dataRoute?.endsWith('/default.rsc') &&
             !existsSync(join(ctx.publishDir, 'server/app', `${key}.html`)):
             return
           case meta.dataRoute?.endsWith('.json'):
-            path = join(ctx.publishDir, 'server/pages', key)
-            value = await buildPagesCacheValue(path)
+            value = await buildPagesCacheValue(join(ctx.publishDir, 'server/pages', key))
             break
           case meta.dataRoute?.endsWith('.rsc'):
-            path = join(ctx.publishDir, 'server/app', key)
-            value = await buildAppCacheValue(path)
+            value = await buildAppCacheValue(join(ctx.publishDir, 'server/app', key))
             break
           case meta.dataRoute === null:
-            path = join(ctx.publishDir, 'server/app', key)
-            value = await buildRouteCacheValue(path)
+            value = await buildRouteCacheValue(join(ctx.publishDir, 'server/app', key))
             break
           default:
             throw new Error(`Unrecognized content: ${route}`)
         }
 
-        await ctx.writeCacheEntry(
-          key,
-          value,
-          meta.dataRoute === null ? `${path}.body` : `${path}.html`,
-        )
+        await writeCacheEntry(key, value, lastModified, ctx)
       }),
     )
 
     // app router 404 pages are not in the prerender manifest
     // so we need to check for them manually
     if (existsSync(join(ctx.publishDir, `server/app/_not-found.html`))) {
+      const lastModified = Date.now()
       const key = '/404'
-      const path = join(ctx.publishDir, 'server/app/_not-found')
-      const value = await buildAppCacheValue(path)
-      await ctx.writeCacheEntry(key, value, `${path}.html`)
+      const value = await buildAppCacheValue(join(ctx.publishDir, 'server/app/_not-found'))
+      await writeCacheEntry(key, value, lastModified, ctx)
     }
   } catch (error) {
     ctx.failBuild('Failed assembling prerendered content for upload', error)
@@ -108,9 +124,10 @@ export const copyFetchContent = async (ctx: PluginContext): Promise<void> => {
 
     await Promise.all(
       paths.map(async (key): Promise<void> => {
+        const lastModified = 1
         const path = join(ctx.publishDir, 'cache/fetch-cache', key)
         const value = await buildFetchCacheValue(path)
-        await ctx.writeCacheEntry(key, value, path)
+        await writeCacheEntry(key, value, lastModified, ctx)
       }),
     )
   } catch (error) {

src/build/plugin-context.ts

@@ -1,8 +1,8 @@
 import { readFileSync } from 'node:fs'
-import { mkdir, readFile, writeFile, stat } from 'node:fs/promises'
+import { readFile } from 'node:fs/promises'
 // Here we need to actually import `resolve` from node:path as we want to resolve the paths
 // eslint-disable-next-line no-restricted-imports
-import { dirname, join, resolve } from 'node:path'
+import { join, resolve } from 'node:path'
 import { fileURLToPath } from 'node:url'
 
 import type {
@@ -14,45 +14,12 @@ import type { PrerenderManifest, RoutesManifest } from 'next/dist/build/index.js
 import type { MiddlewareManifest } from 'next/dist/build/webpack/plugins/middleware-plugin.js'
 import type { NextConfigComplete } from 'next/dist/server/config-shared.js'
 
-import { encodeBlobKey } from '../shared/blobkey.js'
-
 const MODULE_DIR = fileURLToPath(new URL('.', import.meta.url))
 const PLUGIN_DIR = join(MODULE_DIR, '../..')
 
 export const SERVER_HANDLER_NAME = '___netlify-server-handler'
 export const EDGE_HANDLER_NAME = '___netlify-edge-handler'
 
-export type PageCacheValue = {
-  kind: 'PAGE'
-  html: string
-  pageData: string
-  headers?: { [k: string]: string }
-  status?: number
-}
-
-export type RouteCacheValue = {
-  kind: 'ROUTE'
-  body: string
-  headers: { [k: string]: string }
-  status: number
-}
-
-export type FetchCacheValue = {
-  kind: 'FETCH'
-  data: {
-    headers: { [k: string]: string }
-    body: string
-    url: string
-    status?: number
-    tags?: string[]
-  }
-}
-export type CacheValue = PageCacheValue | RouteCacheValue | FetchCacheValue
-export type CacheEntry = {
-  lastModified: number
-  value: CacheValue
-}
-
 export class PluginContext {
   utils: NetlifyPluginUtils
   netlifyConfig: NetlifyPluginOptions['netlifyConfig']
@@ -204,22 +171,6 @@ export class PluginContext {
     return JSON.parse(await readFile(join(this.publishDir, 'routes-manifest.json'), 'utf-8'))
   }
 
-  /**
-   * Write a cache entry to the blob upload directory.
-   */
-  async writeCacheEntry(route: string, value: CacheValue, filePath: string): Promise<void> {
-    // Getting modified file date from prerendered content
-    const { mtime } = await stat(filePath)
-    const path = join(this.blobDir, await encodeBlobKey(route))
-    const entry = JSON.stringify({
-      lastModified: mtime.getTime(),
-      value,
-    } satisfies CacheEntry)
-
-    await mkdir(dirname(path), { recursive: true })
-    await writeFile(path, entry, 'utf-8')
-  }
-
   /** Fails a build with a message and an optional error */
   failBuild(message: string, error?: unknown): never {
     return this.utils.build.failBuild(message, error instanceof Error ? { error } : undefined)

src/index.ts

@@ -3,7 +3,7 @@ import { existsSync } from 'node:fs'
 import type { NetlifyPluginOptions } from '@netlify/build'
 
 import { restoreBuildCache, saveBuildCache } from './build/cache.js'
-import { copyFetchContent, copyPrerenderedContent } from './build/content/prerendered.js'
+import { copyPrerenderedContent } from './build/content/prerendered.js'
 import {
   copyStaticAssets,
   copyStaticContent,
@@ -41,7 +41,6 @@ export const onBuild = async (options: NetlifyPluginOptions) => {
     copyStaticAssets(ctx),
     copyStaticContent(ctx),
     copyPrerenderedContent(ctx),
-    copyFetchContent(ctx),
     createServerHandler(ctx),
     createEdgeHandlers(ctx),
   ])

src/run/handlers/cache.cts

@@ -10,6 +10,7 @@ import { NEXT_CACHE_TAGS_HEADER } from 'next/dist/lib/constants.js'
 import type {
   CacheHandler,
   CacheHandlerContext,
+  CacheHandlerValue,
   IncrementalCache,
 } from 'next/dist/server/lib/incremental-cache/index.js'
 
@@ -43,7 +44,7 @@ export class NetlifyCacheHandler implements CacheHandler {
       span.setAttributes({ key, blobKey })
       const blob = (await this.blobStore.get(blobKey, {
         type: 'json',
-      })) as CacheEntry | null
+      })) as CacheHandlerValue | null
 
       // if blob is null then we don't have a cache entry
       if (!blob) {
@@ -60,33 +61,23 @@ export class NetlifyCacheHandler implements CacheHandler {
         return null
       }
 
-      switch (blob.value.kind) {
+      switch (blob.value?.kind) {
         case 'FETCH':
           span.addEvent('FETCH', { lastModified: blob.lastModified, revalidate: ctx.revalidate })
           span.end()
           return {
             lastModified: blob.lastModified,
-            value: {
-              kind: blob.value.kind,
-              data: blob.value.data,
-              revalidate: blob.value.revalidate,
-            },
+            value: blob.value,
           }
 
         case 'ROUTE':
-          span.addEvent('ROUTE', {
-            lastModified: blob.lastModified,
-            kind: blob.value.kind,
-            status: blob.value.status,
-          })
+          span.addEvent('ROUTE', { lastModified: blob.lastModified, status: blob.value.status })
           span.end()
           return {
             lastModified: blob.lastModified,
             value: {
-              body: Buffer.from(blob.value.body, 'base64'),
-              kind: blob.value.kind,
-              status: blob.value.status,
-              headers: blob.value.headers,
+              ...blob.value,
+              body: Buffer.from(blob.value.body as unknown as string, 'base64'),
             },
           }
         case 'PAGE':
@@ -97,7 +88,7 @@ export class NetlifyCacheHandler implements CacheHandler {
             value: blob.value,
           }
         default:
-          span.recordException(new Error(`Unknown cache entry kind: ${blob.value.kind}`))
+          span.recordException(new Error(`Unknown cache entry kind: ${blob.value?.kind}`))
         // TODO: system level logging not implemented
       }
       span.end()
@@ -147,13 +138,17 @@ export class NetlifyCacheHandler implements CacheHandler {
     })
   }
 
+  /* Not used, but required by the interface */
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  resetRequestCache() {}
+
   /**
    * Checks if a page is stale through on demand revalidated tags
    */
-  private async checkCacheEntryStaleByTags(cacheEntry: CacheEntry, softTags: string[] = []) {
+  private async checkCacheEntryStaleByTags(cacheEntry: CacheHandlerValue, softTags: string[] = []) {
     const tags =
-      'headers' in cacheEntry.value
-        ? cacheEntry.value.headers?.[NEXT_CACHE_TAGS_HEADER]?.split(',') || []
+      cacheEntry.value && 'headers' in cacheEntry.value
+        ? (cacheEntry.value.headers?.[NEXT_CACHE_TAGS_HEADER] as string)?.split(',') || []
         : []
 
     const cacheTags = [...tags, ...softTags]