fix: new ISR cache handling to resolve regression in 13.4 (#2165)

Co-authored-by: Nick Taylor <nick@iamdeveloper.com>

Author: orinokai

packages/runtime/src/helpers/dev.ts

@@ -4,15 +4,11 @@ import type { OnPreBuild } from '@netlify/build'
 import execa from 'execa'
 
 import { writeDevEdgeFunction } from './edge'
-import { patchNextFiles } from './files'
 
 // The types haven't been updated yet
 export const onPreDev: OnPreBuild = async ({ constants, netlifyConfig }) => {
   const base = netlifyConfig.build.base ?? process.cwd()
 
-  // Need to patch the files, because build might not have been run
-  await patchNextFiles(base)
-
   await writeDevEdgeFunction(constants)
   // Don't await this or it will never finish
   execa.node(

packages/runtime/src/helpers/files.ts

@@ -2,18 +2,7 @@ import { cpus } from 'os'
 
 import type { NetlifyConfig } from '@netlify/build'
 import { yellowBright } from 'chalk'
-import {
-  existsSync,
-  readJson,
-  move,
-  copy,
-  writeJson,
-  readFile,
-  writeFile,
-  ensureDir,
-  readFileSync,
-  remove,
-} from 'fs-extra'
+import { existsSync, readJson, move, copy, writeJson, ensureDir, readFileSync, remove } from 'fs-extra'
 import globby from 'globby'
 import { PrerenderManifest } from 'next/dist/build'
 import { outdent } from 'outdent'
@@ -297,45 +286,6 @@ export const moveStaticPages = async ({
   }
 }
 
-const PATCH_WARNING = `/* File patched by Netlify */`
-
-/**
- * Attempt to patch a source file, preserving a backup
- */
-const patchFile = async ({
-  file,
-  replacements,
-}: {
-  file: string
-  replacements: Array<[from: string, to: string]>
-}): Promise<boolean> => {
-  if (!existsSync(file)) {
-    console.warn('File was not found')
-    return false
-  }
-  let content = await readFile(file, 'utf8')
-
-  // If the file has already been patched, patch the backed-up original instead
-  if (content.includes(PATCH_WARNING) && existsSync(`${file}.orig`)) {
-    content = await readFile(`${file}.orig`, 'utf8')
-  }
-
-  const newContent = replacements.reduce((acc, [from, to]) => {
-    if (acc.includes(to)) {
-      console.log('Already patched. Skipping.')
-      return acc
-    }
-    return acc.replace(from, to)
-  }, content)
-  if (newContent === content) {
-    console.warn('File was not changed')
-    return false
-  }
-  await writeFile(`${file}.orig`, content)
-  await writeFile(file, `${newContent}\n${PATCH_WARNING}`)
-  console.log('Done')
-  return true
-}
 /**
  * The file we need has moved around a bit over the past few versions,
  * so we iterate through the options until we find it
@@ -386,82 +336,6 @@ export const getDependenciesOfFile = async (file: string) => {
   return dependencies.files.map((dep) => resolve(dirname(file), dep))
 }
 
-const baseServerReplacements: Array<[string, string]> = [
-  // force manual revalidate during cache fetches
-  // for more info https://github.com/netlify/next-runtime/pull/1541
-  [
-    `checkIsManualRevalidate(req, this.renderOpts.previewProps)`,
-    `checkIsManualRevalidate(process.env._REVALIDATE_SSG ? { headers: { 'x-prerender-revalidate': this.renderOpts.previewProps.previewModeId } } : req, this.renderOpts.previewProps)`,
-  ],
-  // In https://github.com/vercel/next.js/pull/47803 checkIsManualRevalidate was renamed to checkIsOnDemandRevalidate
-  [
-    `checkIsOnDemandRevalidate(req, this.renderOpts.previewProps)`,
-    `checkIsOnDemandRevalidate(process.env._REVALIDATE_SSG ? { headers: { 'x-prerender-revalidate': this.renderOpts.previewProps.previewModeId } } : req, this.renderOpts.previewProps)`,
-  ],
-  // format of checkIsOnDemandRevalidate changed in 13.3.4
-  [
-    'checkIsOnDemandRevalidate)(req, this.renderOpts.previewProps)',
-    'checkIsOnDemandRevalidate)(process.env._REVALIDATE_SSG ? { headers: { "x-prerender-revalidate": this.renderOpts.previewProps.previewModeId } } : req, this.renderOpts.previewProps)',
-  ],
-  // ensure ISR 404 pages send the correct SWR cache headers
-  [`private: isPreviewMode || is404Page && cachedData`, `private: isPreviewMode && cachedData`],
-]
-
-const nextServerReplacements: Array<[string, string]> = [
-  [
-    `getMiddlewareManifest() {\n        if (this.minimalMode) return null;`,
-    `getMiddlewareManifest() {\n        if (this.minimalMode || (!process.env.NETLIFY_DEV && process.env.NEXT_DISABLE_NETLIFY_EDGE !== 'true' && process.env.NEXT_DISABLE_NETLIFY_EDGE !== '1')) return null;`,
-  ],
-  [
-    `generateCatchAllMiddlewareRoute(devReady) {\n        if (this.minimalMode) return []`,
-    `generateCatchAllMiddlewareRoute(devReady) {\n        if (this.minimalMode || (!process.env.NETLIFY_DEV && process.env.NEXT_DISABLE_NETLIFY_EDGE !== 'true' && process.env.NEXT_DISABLE_NETLIFY_EDGE !== '1')) return [];`,
-  ],
-  [
-    `generateCatchAllMiddlewareRoute() {\n        if (this.minimalMode) return undefined;`,
-    `generateCatchAllMiddlewareRoute() {\n        if (this.minimalMode || (!process.env.NETLIFY_DEV && process.env.NEXT_DISABLE_NETLIFY_EDGE !== 'true' && process.env.NEXT_DISABLE_NETLIFY_EDGE !== '1')) return undefined;`,
-  ],
-  [
-    `getMiddlewareManifest() {\n        if (this.minimalMode) {`,
-    `getMiddlewareManifest() {\n        if (!this.minimalMode && (!process.env.NETLIFY_DEV && process.env.NEXT_DISABLE_NETLIFY_EDGE === 'true' || process.env.NEXT_DISABLE_NETLIFY_EDGE === '1')) {`,
-  ],
-]
-
-export const patchNextFiles = async (root: string): Promise<void> => {
-  const baseServerFile = getServerFile(root)
-  console.log(`Patching ${baseServerFile}`)
-  if (baseServerFile) {
-    await patchFile({
-      file: baseServerFile,
-      replacements: baseServerReplacements,
-    })
-  }
-
-  const nextServerFile = getServerFile(root, false)
-  console.log(`Patching ${nextServerFile}`)
-  if (nextServerFile) {
-    await patchFile({
-      file: nextServerFile,
-      replacements: nextServerReplacements,
-    })
-  }
-}
-
-export const unpatchFile = async (file: string): Promise<void> => {
-  const origFile = `${file}.orig`
-  if (existsSync(origFile)) {
-    await move(origFile, file, { overwrite: true })
-  }
-}
-
-export const unpatchNextFiles = async (root: string): Promise<void> => {
-  const baseServerFile = getServerFile(root)
-  await unpatchFile(baseServerFile)
-  const nextServerFile = getServerFile(root, false)
-  if (nextServerFile !== baseServerFile) {
-    await unpatchFile(nextServerFile)
-  }
-}
-
 export const movePublicFiles = async ({
   appDir,
   outdir,

packages/runtime/src/index.ts

@@ -17,7 +17,7 @@ import {
 } from './helpers/config'
 import { onPreDev } from './helpers/dev'
 import { writeEdgeFunctions, loadMiddlewareManifest, cleanupEdgeFunctions } from './helpers/edge'
-import { moveStaticPages, movePublicFiles, patchNextFiles, removeMetadataFiles } from './helpers/files'
+import { moveStaticPages, movePublicFiles, removeMetadataFiles } from './helpers/files'
 import { splitApiRoutes } from './helpers/flags'
 import {
   generateFunctions,
@@ -184,8 +184,6 @@ const plugin: NetlifyPlugin = {
 
     await movePublicFiles({ appDir, outdir, publish, basePath })
 
-    await patchNextFiles(appDir)
-
     if (!destr(process.env.SERVE_STATIC_FILES_FROM_ORIGIN)) {
       await moveStaticPages({ target, netlifyConfig, i18n, basePath })
     }

packages/runtime/src/templates/getHandler.ts

@@ -69,9 +69,6 @@ const makeHandler = ({ conf, app, pageRoot, NextServer, staticManifest = [], mod
 
   // We don't want to write ISR files to disk in the lambda environment
   conf.experimental.isrFlushToDisk = false
-  // This is our flag that we use when patching the source
-  // eslint-disable-next-line no-underscore-dangle
-  process.env._REVALIDATE_SSG = 'true'
   for (const [key, value] of Object.entries(conf.env)) {
     process.env[key] = String(value)
   }

packages/runtime/src/templates/server.ts

@@ -1,6 +1,6 @@
 import { PrerenderManifest } from 'next/dist/build'
 import type { BaseNextResponse } from 'next/dist/server/base-http'
-import { NodeRequestHandler, Options } from 'next/dist/server/next-server'
+import type { NodeRequestHandler, Options } from 'next/dist/server/next-server'
 
 import {
   netlifyApiFetch,
@@ -42,6 +42,7 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
         // conditionally use the prebundled React module
         this.netlifyPrebundleReact(url)
 
+        // intercept on-demand revalidation requests and handle with the Netlify API
         if (headers['x-prerender-revalidate'] && this.netlifyConfig.revalidateToken) {
           // handle on-demand revalidation by purging the ODB cache
           await this.netlifyRevalidate(url)
@@ -50,9 +51,20 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
           res.statusCode = 200
           res.setHeader('x-nextjs-cache', 'REVALIDATED')
           res.send()
-        } else {
-          return handler(req, res, parsedUrl)
+          return
         }
+
+        // force Next to revalidate all requests so that we always have fresh content
+        // for our ODBs and middleware is disabled at the origin
+        // but ignore in preview mode (prerender_bypass is set to true in preview mode)
+        // because otherwise revalidate will override preview mode
+        if (!headers.cookie?.includes('__prerender_bypass')) {
+          // this header controls whether Next.js will revalidate the page
+          // and needs to be set to the preview mode id to enable it
+          headers['x-prerender-revalidate'] = this.renderOpts.previewProps.previewModeId
+        }
+
+        return handler(req, res, parsedUrl)
       }
     }
 

test/helpers/files.spec.ts

@@ -8,8 +8,6 @@ import {
   stripLocale,
   matchesRedirect,
   matchesRewrite,
-  patchNextFiles,
-  unpatchNextFiles,
   getDependenciesOfFile,
   getSourceFileForPage,
 } from '../../packages/runtime/src/helpers/files'
@@ -189,28 +187,6 @@ describe('files utility functions', () => {
   })
 })
 
-describeCwdTmpDir('file patching', () => {
-  it('patches Next server files', async () => {
-    // Testing to make sure that the patching functionality works within base-server.js and next-server.js files
-    const root = path.resolve(dirname(resolve(__dirname, '..')))
-    await copy(join(root, 'package.json'), path.join(process.cwd(), 'package.json'))
-    await ensureDir(path.join(process.cwd(), 'node_modules'))
-    await copy(path.join(root, 'node_modules', 'next'), path.join(process.cwd(), 'node_modules', 'next'))
-
-    await patchNextFiles(process.cwd())
-    const serverFile = path.resolve(process.cwd(), 'node_modules', 'next', 'dist', 'server', 'base-server.js')
-    const patchedData = await readFileSync(serverFile, 'utf8')
-    expect(patchedData.includes('_REVALIDATE_SSG')).toBeTruthy()
-    expect(patchedData.includes('private: isPreviewMode && cachedData')).toBeTruthy()
-
-    await unpatchNextFiles(process.cwd())
-
-    const unPatchedData = await readFileSync(serverFile, 'utf8')
-    expect(unPatchedData.includes('_REVALIDATE_SSG')).toBeFalsy()
-    expect(unPatchedData.includes('private: isPreviewMode && cachedData')).toBeFalsy()
-  })
-})
-
 describe('dependency tracing', () => {
   it('generates dependency list from a source file', async () => {
     const dependencies = await getDependenciesOfFile(resolve(__dirname, '../fixtures/analysis/background.js'))