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

Handle de-duping revalidations in minimal mode #34935

Merged
merged 8 commits into from Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/next/server/base-server.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -1277,7 +1280,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
Expand Down
3 changes: 2 additions & 1 deletion packages/next/server/next-server.ts
Expand Up @@ -120,7 +120,8 @@ export default class NextNodeServer extends BaseServer {
new ImageOptimizerCache({
distDir: this.distDir,
nextConfig: this.nextConfig,
})
}),
this.minimalMode
)
}

Expand Down
65 changes: 51 additions & 14 deletions packages/next/server/response-cache.ts
Expand Up @@ -79,16 +79,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) {
Expand Down Expand Up @@ -119,12 +128,27 @@ 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 () => {
try {
const cachedResponse = key ? await this.incrementalCache.get(key) : null
const cachedResponse =
key && !this.minimalMode ? await this.incrementalCache.get(key) : null

if (cachedResponse && !context.isManualRevalidate) {
resolve({
isStale: cachedResponse.isStale,
Expand Down Expand Up @@ -156,17 +180,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) {
// while revalidating in the background we can't reject as
Expand Down
Expand Up @@ -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,
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 3 additions & 0 deletions test/production/required-server-files-i18n.test.ts
Expand Up @@ -11,6 +11,7 @@ import {
initNextServerScript,
killApp,
renderViaHTTP,
waitFor,
} from 'next-test-utils'

describe('should set-up next', () => {
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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())
Expand Down
26 changes: 26 additions & 0 deletions test/production/required-server-files.test.ts
Expand Up @@ -11,6 +11,7 @@ import {
initNextServerScript,
killApp,
renderViaHTTP,
waitFor,
} from 'next-test-utils'

describe('should set-up next', () => {
Expand Down Expand Up @@ -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, {
Expand All @@ -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, {
Expand Down Expand Up @@ -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())
Expand Down
4 changes: 4 additions & 0 deletions 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 {
Expand All @@ -18,6 +21,7 @@ export async function getStaticProps() {
props: {
hello: 'world',
data,
gspCalls,
},
revalidate: 1,
}
Expand Down