Handle de-duping revalidations in minimal mode (#34935)

* Handle de-duping revalidations in minimal mode * fix type * remove header check * update other tests * update test
vercel · Mar 2, 2022 · 81c50f4 · 81c50f4
1 parent e51d262
commit 81c50f4
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 18 deletions.
diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts
@@ -373,7 +373,10 @@ export default abstract class Server {
         }
       },
     })
-    this.responseCache = new ResponseCache(this.incrementalCache)
+    this.responseCache = new ResponseCache(
+      this.incrementalCache,
+      this.minimalMode
+    )
   }
 
   public logError(err: Error): void {
@@ -1274,7 +1277,7 @@ export default abstract class Server {
     }
 
     let ssgCacheKey =
-      isPreviewMode || !isSSG || this.minimalMode || opts.supportsDynamicHTML
+      isPreviewMode || !isSSG || opts.supportsDynamicHTML
         ? null // Preview mode and manual revalidate bypasses the cache
         : `${locale ? `/${locale}` : ''}${
             (pathname === '/' || resolvedUrlPathname === '/') && locale

diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts
@@ -120,7 +120,8 @@ export default class NextNodeServer extends BaseServer {
         new ImageOptimizerCache({
           distDir: this.distDir,
           nextConfig: this.nextConfig,
-        })
+        }),
+        this.minimalMode
       )
     }
 

diff --git a/packages/next/server/response-cache.ts b/packages/next/server/response-cache.ts
@@ -81,16 +81,25 @@ interface IncrementalCache {
 export default class ResponseCache {
   incrementalCache: IncrementalCache
   pendingResponses: Map<string, Promise<ResponseCacheEntry | null>>
+  previousCacheItem?: {
+    key: string
+    entry: ResponseCacheEntry | null
+    expiresAt: number
+  }
+  minimalMode?: boolean
 
-  constructor(incrementalCache: IncrementalCache) {
+  constructor(incrementalCache: IncrementalCache, minimalMode: boolean) {
     this.incrementalCache = incrementalCache
     this.pendingResponses = new Map()
+    this.minimalMode = minimalMode
   }
 
   public get(
     key: string | null,
     responseGenerator: ResponseGenerator,
-    context: { isManualRevalidate?: boolean }
+    context: {
+      isManualRevalidate?: boolean
+    }
   ): Promise<ResponseCacheEntry | null> {
     const pendingResponse = key ? this.pendingResponses.get(key) : null
     if (pendingResponse) {
@@ -121,13 +130,28 @@ export default class ResponseCache {
       }
     }
 
+    // we keep the previous cache entry around to leverage
+    // when the incremental cache is disabled in minimal mode
+    if (
+      key &&
+      this.minimalMode &&
+      this.previousCacheItem?.key === key &&
+      this.previousCacheItem.expiresAt > Date.now()
+    ) {
+      resolve(this.previousCacheItem.entry)
+      this.pendingResponses.delete(key)
+      return promise
+    }
+
     // We wait to do any async work until after we've added our promise to
     // `pendingResponses` to ensure that any any other calls will reuse the
     // same promise until we've fully finished our work.
     ;(async () => {
       let cachedResponse: IncrementalCacheItem = null
       try {
-        cachedResponse = key ? await this.incrementalCache.get(key) : null
+        cachedResponse =
+          key && !this.minimalMode ? await this.incrementalCache.get(key) : null
+
         if (cachedResponse && !context.isManualRevalidate) {
           resolve({
             isStale: cachedResponse.isStale,
@@ -159,17 +183,30 @@ export default class ResponseCache {
         )
 
         if (key && cacheEntry && typeof cacheEntry.revalidate !== 'undefined') {
-          await this.incrementalCache.set(
-            key,
-            cacheEntry.value?.kind === 'PAGE'
-              ? {
-                  kind: 'PAGE',
-                  html: cacheEntry.value.html.toUnchunkedString(),
-                  pageData: cacheEntry.value.pageData,
-                }
-              : cacheEntry.value,
-            cacheEntry.revalidate
-          )
+          if (this.minimalMode) {
+            this.previousCacheItem = {
+              key,
+              entry: cacheEntry,
+              expiresAt:
+                typeof cacheEntry.revalidate !== 'number'
+                  ? Date.now() + 1000
+                  : Date.now() + cacheEntry?.revalidate * 1000,
+            }
+          } else {
+            await this.incrementalCache.set(
+              key,
+              cacheEntry.value?.kind === 'PAGE'
+                ? {
+                    kind: 'PAGE',
+                    html: cacheEntry.value.html.toUnchunkedString(),
+                    pageData: cacheEntry.value.pageData,
+                  }
+                : cacheEntry.value,
+              cacheEntry.revalidate
+            )
+          }
+        } else {
+          this.previousCacheItem = undefined
         }
       } catch (err) {
         // when a getStaticProps path is erroring we automatically re-set the

diff --git a/test/e2e/prerender.test.ts b/test/e2e/prerender.test.ts
@@ -164,6 +164,11 @@ describe('Prerender', () => {
       initialRevalidateSeconds: 1,
       srcRoute: '/blocking-fallback-some/[slug]',
     },
+    '/blocking-fallback/test-errors-1': {
+      dataRoute: `/_next/data/${next.buildId}/blocking-fallback/test-errors-1.json`,
+      initialRevalidateSeconds: 1,
+      srcRoute: '/blocking-fallback/[slug]',
+    },
     '/blog': {
       dataRoute: `/_next/data/${next.buildId}/blog.json`,
       initialRevalidateSeconds: 10,

diff --git a/test/integration/required-server-files-ssr-404/test/index.test.js b/test/integration/required-server-files-ssr-404/test/index.test.js
@@ -4,7 +4,7 @@ import http from 'http'
 import fs from 'fs-extra'
 import { join } from 'path'
 import cheerio from 'cheerio'
-import { nextServer } from 'next-test-utils'
+import { nextServer, waitFor } from 'next-test-utils'
 import {
   fetchViaHTTP,
   findPort,
@@ -140,6 +140,7 @@ describe('Required Server Files', () => {
     expect($('#slug').text()).toBe('first')
     expect(data.hello).toBe('world')
 
+    await waitFor(2000)
     const html2 = await renderViaHTTP(appPort, '/fallback/first')
     const $2 = cheerio.load(html2)
     const data2 = JSON.parse($2('#props').text())

diff --git a/test/production/required-server-files-i18n.test.ts b/test/production/required-server-files-i18n.test.ts
@@ -11,6 +11,7 @@ import {
   initNextServerScript,
   killApp,
   renderViaHTTP,
+  waitFor,
 } from 'next-test-utils'
 
 describe('should set-up next', () => {
@@ -133,6 +134,7 @@ describe('should set-up next', () => {
       's-maxage=1, stale-while-revalidate'
     )
 
+    await waitFor(2000)
     await next.patchFile('standalone/data.txt', 'hide')
 
     const res2 = await fetchViaHTTP(appPort, '/gsp', undefined, {
@@ -211,6 +213,7 @@ describe('should set-up next', () => {
     expect($('#slug').text()).toBe('first')
     expect(data.hello).toBe('world')
 
+    await waitFor(2000)
     const html2 = await renderViaHTTP(appPort, '/fallback/first')
     const $2 = cheerio.load(html2)
     const data2 = JSON.parse($2('#props').text())

diff --git a/test/production/required-server-files.test.ts b/test/production/required-server-files.test.ts
@@ -11,6 +11,7 @@ import {
   initNextServerScript,
   killApp,
   renderViaHTTP,
+  waitFor,
 } from 'next-test-utils'
 
 describe('should set-up next', () => {
@@ -155,7 +156,30 @@ describe('should set-up next', () => {
     expect(typeof requiredFilesManifest.appDir).toBe('string')
   })
 
+  it('should de-dupe HTML/data requests', async () => {
+    const res = await fetchViaHTTP(appPort, '/gsp', undefined, {
+      redirect: 'manual',
+    })
+    expect(res.status).toBe(200)
+    const $ = cheerio.load(await res.text())
+    const props = JSON.parse($('#props').text())
+    expect(props.gspCalls).toBeDefined()
+
+    const res2 = await fetchViaHTTP(
+      appPort,
+      `/_next/data/${next.buildId}/gsp.json`,
+      undefined,
+      {
+        redirect: 'manual',
+      }
+    )
+    expect(res2.status).toBe(200)
+    const { pageProps: props2 } = await res2.json()
+    expect(props2.gspCalls).toBe(props.gspCalls)
+  })
+
   it('should set correct SWR headers with notFound gsp', async () => {
+    await waitFor(2000)
     await next.patchFile('standalone/data.txt', 'show')
 
     const res = await fetchViaHTTP(appPort, '/gsp', undefined, {
@@ -166,6 +190,7 @@ describe('should set-up next', () => {
       's-maxage=1, stale-while-revalidate'
     )
 
+    await waitFor(2000)
     await next.patchFile('standalone/data.txt', 'hide')
 
     const res2 = await fetchViaHTTP(appPort, '/gsp', undefined, {
@@ -244,6 +269,7 @@ describe('should set-up next', () => {
     expect($('#slug').text()).toBe('first')
     expect(data.hello).toBe('world')
 
+    await waitFor(2000)
     const html2 = await renderViaHTTP(appPort, '/fallback/first')
     const $2 = cheerio.load(html2)
     const data2 = JSON.parse($2('#props').text())

diff --git a/test/production/required-server-files/pages/gsp.js b/test/production/required-server-files/pages/gsp.js
@@ -1,11 +1,14 @@
 import fs from 'fs'
 import path from 'path'
 
+let gspCalls = 0
+
 export async function getStaticProps() {
   const data = await fs.promises.readFile(
     path.join(process.cwd(), 'data.txt'),
     'utf8'
   )
+  gspCalls += 1
 
   if (data.trim() === 'hide') {
     return {
@@ -18,6 +21,7 @@ export async function getStaticProps() {
     props: {
       hello: 'world',
       data,
+      gspCalls,
     },
     revalidate: 1,
   }