From 5792533781e6d6fdf10d5b400d8e97d25bd5edc3 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 23 Feb 2023 01:19:59 -0800 Subject: [PATCH] Update app cache handler loading (#46290 Follow-up to https://github.com/vercel/next.js/pull/46287 this updates how we load the cache handler for the incremental cache so it's compatible with edge and also adds regression testing with a custom handler. --- packages/next/src/build/entries.ts | 2 + packages/next/src/build/index.ts | 7 + .../loaders/next-edge-ssr-loader/index.ts | 9 ++ .../loaders/next-edge-ssr-loader/render.ts | 3 + packages/next/src/export/index.ts | 2 + packages/next/src/export/worker.ts | 10 ++ .../src/lib/create-router-client-filter.ts | 2 + .../src/server/lib/incremental-cache/index.ts | 27 ++-- packages/next/src/server/next-server.ts | 12 +- packages/next/src/server/web-server.ts | 5 +- .../app-static-custom-handler.test.ts | 2 + .../e2e/app-dir/app-static/app-static.test.ts | 151 +++++++++++------- test/e2e/app-dir/app-static/cache-handler.js | 22 +++ test/e2e/app-dir/app-static/next.config.js | 3 + 14 files changed, 175 insertions(+), 82 deletions(-) create mode 100644 test/e2e/app-dir/app-static/app-static-custom-handler.test.ts create mode 100644 test/e2e/app-dir/app-static/cache-handler.js diff --git a/packages/next/src/build/entries.ts b/packages/next/src/build/entries.ts index c8a92af831..a5ed16809b 100644 --- a/packages/next/src/build/entries.ts +++ b/packages/next/src/build/entries.ts @@ -225,6 +225,8 @@ export function getEdgeServerEntry(opts: { pagesType: opts.pagesType, appDirLoader: Buffer.from(opts.appDirLoader || '').toString('base64'), sriEnabled: !opts.isDev && !!opts.config.experimental.sri?.algorithm, + incrementalCacheHandlerPath: + opts.config.experimental.incrementalCacheHandlerPath, } return { diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index d0308a4e15..6b83bed28e 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -913,6 +913,13 @@ export default async function build( experimental: { ...config.experimental, trustHostHeader: ciEnvironment.hasNextSupport, + incrementalCacheHandlerPath: config.experimental + .incrementalCacheHandlerPath + ? path.relative( + distDir, + config.experimental.incrementalCacheHandlerPath + ) + : undefined, }, }, appDir: dir, diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts index d94b1671b2..b8e7f33837 100644 --- a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts +++ b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts @@ -15,6 +15,7 @@ export type EdgeSSRLoaderQuery = { appDirLoader?: string pagesType: 'app' | 'pages' | 'root' sriEnabled: boolean + incrementalCacheHandlerPath?: string } /* @@ -44,6 +45,7 @@ export default async function edgeSSRLoader(this: any) { appDirLoader: appDirLoaderBase64, pagesType, sriEnabled, + incrementalCacheHandlerPath, } = this.getOptions() const appDirLoader = Buffer.from( @@ -117,6 +119,12 @@ export default async function edgeSSRLoader(this: any) { const appRenderToHTML = null ` } + + const incrementalCacheHandler = ${ + incrementalCacheHandlerPath + ? `require("${incrementalCacheHandlerPath}")` + : 'null' + } const buildManifest = self.__BUILD_MANIFEST const reactLoadableManifest = self.__REACT_LOADABLE_MANIFEST @@ -146,6 +154,7 @@ export default async function edgeSSRLoader(this: any) { config: ${stringifiedConfig}, buildId: ${JSON.stringify(buildId)}, fontLoaderManifest, + incrementalCacheHandler, }) export const ComponentMod = pageMod diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts index 10f3f1f041..87d4b7c2a1 100644 --- a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts +++ b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts @@ -31,6 +31,7 @@ export function getRender({ config, buildId, fontLoaderManifest, + incrementalCacheHandler, }: { pagesType: 'app' | 'pages' | 'root' dev: boolean @@ -51,6 +52,7 @@ export function getRender({ config: NextConfigComplete buildId: string fontLoaderManifest: FontLoaderManifest + incrementalCacheHandler?: any }) { const isAppPath = pagesType === 'app' const baseLoadComponentResult = { @@ -80,6 +82,7 @@ export function getRender({ }, appRenderToHTML, pagesRenderToHTML, + incrementalCacheHandler, loadComponent: async (pathname) => { if (isAppPath) return null diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts index ec4108c62d..9d008d6c8a 100644 --- a/packages/next/src/export/index.ts +++ b/packages/next/src/export/index.ts @@ -653,6 +653,8 @@ export default async function exportApp( debugOutput: options.debugOutput, isrMemoryCacheSize: nextConfig.experimental.isrMemoryCacheSize, fetchCache: nextConfig.experimental.appDir, + incrementalCacheHandlerPath: + nextConfig.experimental.incrementalCacheHandlerPath, }) for (const validation of result.ampValidations || []) { diff --git a/packages/next/src/export/worker.ts b/packages/next/src/export/worker.ts index 3cee3c3a76..2f484e3077 100644 --- a/packages/next/src/export/worker.ts +++ b/packages/next/src/export/worker.ts @@ -84,6 +84,7 @@ interface ExportPageInput { debugOutput?: boolean isrMemoryCacheSize?: NextConfigComplete['experimental']['isrMemoryCacheSize'] fetchCache?: boolean + incrementalCacheHandlerPath?: string } interface ExportPageResults { @@ -141,6 +142,7 @@ export default async function exportPage({ debugOutput, isrMemoryCacheSize, fetchCache, + incrementalCacheHandlerPath, }: ExportPageInput): Promise { setHttpClientAndAgentOptions({ httpAgentOptions, @@ -329,6 +331,13 @@ export default async function exportPage({ // only fully static paths are fully generated here if (isAppDir) { if (fetchCache) { + let CacheHandler: any + + if (incrementalCacheHandlerPath) { + CacheHandler = require(incrementalCacheHandlerPath) + CacheHandler = CacheHandler.default || CacheHandler + } + curRenderOpts.incrementalCache = new IncrementalCache({ dev: false, requestHeaders: {}, @@ -353,6 +362,7 @@ export default async function exportPage({ stat: (f) => fs.promises.stat(f), }, serverDistDir: join(distDir, 'server'), + CurCacheHandler: CacheHandler, }) } diff --git a/packages/next/src/lib/create-router-client-filter.ts b/packages/next/src/lib/create-router-client-filter.ts index d81df08a8a..6b5ad1510b 100644 --- a/packages/next/src/lib/create-router-client-filter.ts +++ b/packages/next/src/lib/create-router-client-filter.ts @@ -20,6 +20,8 @@ export function createClientRouterFilter( let subPath = '' const pathParts = path.split('/') + // start at 1 since we split on '/' and the path starts + // with this so the first entry is an empty string for (let i = 1; i < pathParts.length + 1; i++) { const curPart = pathParts[i] diff --git a/packages/next/src/server/lib/incremental-cache/index.ts b/packages/next/src/server/lib/incremental-cache/index.ts index 8dc617087f..c19e57b7c0 100644 --- a/packages/next/src/server/lib/incremental-cache/index.ts +++ b/packages/next/src/server/lib/incremental-cache/index.ts @@ -67,8 +67,8 @@ export class IncrementalCache { requestHeaders, maxMemoryCacheSize, getPrerenderManifest, - incrementalCacheHandlerPath, fetchCacheKeyPrefix, + CurCacheHandler, }: { fs?: CacheFs dev: boolean @@ -79,23 +79,18 @@ export class IncrementalCache { flushToDisk?: boolean requestHeaders: IncrementalCache['requestHeaders'] maxMemoryCacheSize?: number - incrementalCacheHandlerPath?: string getPrerenderManifest: () => PrerenderManifest fetchCacheKeyPrefix?: string + CurCacheHandler?: typeof CacheHandler }) { - let cacheHandlerMod: any + if (!CurCacheHandler) { + if (fs && serverDistDir) { + CurCacheHandler = FileSystemCache + } - if (fs && serverDistDir) { - cacheHandlerMod = FileSystemCache - } - - if (process.env.NEXT_RUNTIME !== 'edge' && incrementalCacheHandlerPath) { - cacheHandlerMod = require(incrementalCacheHandlerPath) - cacheHandlerMod = cacheHandlerMod.default || cacheHandlerMod - } - - if (!incrementalCacheHandlerPath && minimalMode && fetchCache) { - cacheHandlerMod = FetchCache + if (minimalMode && fetchCache) { + CurCacheHandler = FetchCache + } } if (process.env.__NEXT_TEST_MAX_ISR_CACHE) { @@ -107,8 +102,8 @@ export class IncrementalCache { this.requestHeaders = requestHeaders this.prerenderManifest = getPrerenderManifest() - if (cacheHandlerMod) { - this.cacheHandler = new (cacheHandlerMod as typeof CacheHandler)({ + if (CurCacheHandler) { + this.cacheHandler = new CurCacheHandler({ dev, fs, flushToDisk, diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index 8bae039b39..053b53187e 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -277,6 +277,15 @@ export default class NextNodeServer extends BaseServer { requestHeaders: IncrementalCache['requestHeaders'] }) { const dev = !!this.renderOpts.dev + let CacheHandler: any + const { incrementalCacheHandlerPath } = this.nextConfig.experimental + + if (incrementalCacheHandlerPath) { + CacheHandler = require(this.minimalMode + ? join(this.distDir, incrementalCacheHandlerPath) + : incrementalCacheHandlerPath) + CacheHandler = CacheHandler.default || CacheHandler + } // incremental-cache is request specific with a shared // although can have shared caches in module scope // per-cache handler @@ -292,8 +301,6 @@ export default class NextNodeServer extends BaseServer { maxMemoryCacheSize: this.nextConfig.experimental.isrMemoryCacheSize, flushToDisk: !this.minimalMode && this.nextConfig.experimental.isrFlushToDisk, - incrementalCacheHandlerPath: - this.nextConfig.experimental?.incrementalCacheHandlerPath, getPrerenderManifest: () => { if (dev) { return { @@ -307,6 +314,7 @@ export default class NextNodeServer extends BaseServer { return this.getPrerenderManifest() } }, + CurCacheHandler: CacheHandler, }) } diff --git a/packages/next/src/server/web-server.ts b/packages/next/src/server/web-server.ts index 514df48b8f..e4330f4f29 100644 --- a/packages/next/src/server/web-server.ts +++ b/packages/next/src/server/web-server.ts @@ -38,6 +38,7 @@ interface WebServerOptions extends Options { Pick pagesRenderToHTML?: typeof import('./render').renderToHTML appRenderToHTML?: typeof import('./app-render').renderToHTMLOrFlight + incrementalCacheHandler?: any } } @@ -71,8 +72,8 @@ export default class NextWebServer extends BaseServer { fetchCacheKeyPrefix: this.nextConfig.experimental.fetchCacheKeyPrefix, maxMemoryCacheSize: this.nextConfig.experimental.isrMemoryCacheSize, flushToDisk: false, - incrementalCacheHandlerPath: - this.nextConfig.experimental?.incrementalCacheHandlerPath, + CurCacheHandler: + this.serverOptions.webServerConfig.incrementalCacheHandler, getPrerenderManifest: () => { if (dev) { return { diff --git a/test/e2e/app-dir/app-static/app-static-custom-handler.test.ts b/test/e2e/app-dir/app-static/app-static-custom-handler.test.ts new file mode 100644 index 0000000000..7092b09389 --- /dev/null +++ b/test/e2e/app-dir/app-static/app-static-custom-handler.test.ts @@ -0,0 +1,2 @@ +process.env.CUSTOM_CACHE_HANDLER = '1' +require('./app-static.test') diff --git a/test/e2e/app-dir/app-static/app-static.test.ts b/test/e2e/app-dir/app-static/app-static.test.ts index a8911d0a46..503615d968 100644 --- a/test/e2e/app-dir/app-static/app-static.test.ts +++ b/test/e2e/app-dir/app-static/app-static.test.ts @@ -14,9 +14,10 @@ createNextDescribe( files: __dirname, env: { NEXT_DEBUG_BUILD: '1', + CUSTOM_CACHE_HANDLER: process.env.CUSTOM_CACHE_HANDLER, }, }, - ({ next, isNextDev: isDev, isNextStart }) => { + ({ next, isNextDev: isDev, isNextStart, isNextDeploy }) => { if (isNextStart) { it('should output HTML/RSC files for static paths', async () => { const files = ( @@ -383,19 +384,23 @@ createNextDescribe( const html = await res.text() const $ = cheerio.load(html) - const layoutData = $('#layout-data').text() - const pageData = $('#page-data').text() + // the test cache handler is simple and doesn't share + // state across workers so not guaranteed to have cache hit + if (!(isNextDeploy && process.env.CUSTOM_CACHE_HANDLER)) { + const layoutData = $('#layout-data').text() + const pageData = $('#page-data').text() - const res2 = await fetchViaHTTP( - next.url, - '/variable-revalidate-edge/revalidate-3' - ) - expect(res2.status).toBe(200) - const html2 = await res2.text() - const $2 = cheerio.load(html2) + const res2 = await fetchViaHTTP( + next.url, + '/variable-revalidate-edge/revalidate-3' + ) + expect(res2.status).toBe(200) + const html2 = await res2.text() + const $2 = cheerio.load(html2) - expect($2('#layout-data').text()).toBe(layoutData) - expect($2('#page-data').text()).toBe(pageData) + expect($2('#layout-data').text()).toBe(layoutData) + expect($2('#page-data').text()).toBe(pageData) + } return 'success' }, 'success') }) @@ -584,32 +589,36 @@ createNextDescribe( } }) - it('should handle dynamicParams: false correctly', async () => { - const validParams = ['tim', 'seb', 'styfle'] + // since we aren't leveraging fs cache with custom handler + // then these will 404 as they are cache misses + if (!(isNextStart && process.env.CUSTOM_CACHE_HANDLER)) { + it('should handle dynamicParams: false correctly', async () => { + const validParams = ['tim', 'seb', 'styfle'] - for (const param of validParams) { - const res = await next.fetch(`/blog/${param}`, { - redirect: 'manual', - }) - expect(res.status).toBe(200) - const html = await res.text() - const $ = cheerio.load(html) + for (const param of validParams) { + const res = await next.fetch(`/blog/${param}`, { + redirect: 'manual', + }) + expect(res.status).toBe(200) + const html = await res.text() + const $ = cheerio.load(html) - expect(JSON.parse($('#params').text())).toEqual({ - author: param, - }) - expect($('#page').text()).toBe('/blog/[author]') - } - const invalidParams = ['timm', 'non-existent'] + expect(JSON.parse($('#params').text())).toEqual({ + author: param, + }) + expect($('#page').text()).toBe('/blog/[author]') + } + const invalidParams = ['timm', 'non-existent'] - for (const param of invalidParams) { - const invalidRes = await next.fetch(`/blog/${param}`, { - redirect: 'manual', - }) - expect(invalidRes.status).toBe(404) - expect(await invalidRes.text()).toContain('page could not be found') - } - }) + for (const param of invalidParams) { + const invalidRes = await next.fetch(`/blog/${param}`, { + redirect: 'manual', + }) + expect(invalidRes.status).toBe(404) + expect(await invalidRes.text()).toContain('page could not be found') + } + }) + } it('should work with forced dynamic path', async () => { for (const slug of ['first', 'second']) { @@ -664,40 +673,50 @@ createNextDescribe( } }) - it('should navigate to static path correctly', async () => { - const browser = await next.browser('/blog/tim') - await browser.eval('window.beforeNav = 1') + // since we aren't leveraging fs cache with custom handler + // then these will 404 as they are cache misses + if (!(isNextStart && process.env.CUSTOM_CACHE_HANDLER)) { + it('should navigate to static path correctly', async () => { + const browser = await next.browser('/blog/tim') + await browser.eval('window.beforeNav = 1') - expect( - await browser.eval('document.documentElement.innerHTML') - ).toContain('/blog/[author]') - await browser.elementByCss('#author-2').click() + expect( + await browser.eval('document.documentElement.innerHTML') + ).toContain('/blog/[author]') + await browser.elementByCss('#author-2').click() - await check(async () => { - const params = JSON.parse(await browser.elementByCss('#params').text()) - return params.author === 'seb' ? 'found' : params - }, 'found') + await check(async () => { + const params = JSON.parse( + await browser.elementByCss('#params').text() + ) + return params.author === 'seb' ? 'found' : params + }, 'found') - expect(await browser.eval('window.beforeNav')).toBe(1) - await browser.elementByCss('#author-1-post-1').click() + expect(await browser.eval('window.beforeNav')).toBe(1) + await browser.elementByCss('#author-1-post-1').click() - await check(async () => { - const params = JSON.parse(await browser.elementByCss('#params').text()) - return params.author === 'tim' && params.slug === 'first-post' - ? 'found' - : params - }, 'found') + await check(async () => { + const params = JSON.parse( + await browser.elementByCss('#params').text() + ) + return params.author === 'tim' && params.slug === 'first-post' + ? 'found' + : params + }, 'found') - expect(await browser.eval('window.beforeNav')).toBe(1) - await browser.back() + expect(await browser.eval('window.beforeNav')).toBe(1) + await browser.back() - await check(async () => { - const params = JSON.parse(await browser.elementByCss('#params').text()) - return params.author === 'seb' ? 'found' : params - }, 'found') + await check(async () => { + const params = JSON.parse( + await browser.elementByCss('#params').text() + ) + return params.author === 'seb' ? 'found' : params + }, 'found') - expect(await browser.eval('window.beforeNav')).toBe(1) - }) + expect(await browser.eval('window.beforeNav')).toBe(1) + }) + } it('should ssr dynamically when detected automatically with fetch cache option', async () => { const pathname = '/ssr-auto/cache-no-store' @@ -959,5 +978,13 @@ createNextDescribe( await waitFor(1000) checkUrl() }) + + if (process.env.CUSTOM_CACHE_HANDLER && !isNextDeploy) { + it('should have logs from cache-handler', () => { + expect(next.cliOutput).toContain('initialized custom cache-handler') + expect(next.cliOutput).toContain('cache-handler get') + expect(next.cliOutput).toContain('cache-handler set') + }) + } } ) diff --git a/test/e2e/app-dir/app-static/cache-handler.js b/test/e2e/app-dir/app-static/cache-handler.js new file mode 100644 index 0000000000..4b822c9c46 --- /dev/null +++ b/test/e2e/app-dir/app-static/cache-handler.js @@ -0,0 +1,22 @@ +const cache = new Map() + +module.exports = class CacheHandler { + constructor(options) { + this.options = options + this.cache = {} + console.log('initialized custom cache-handler') + } + + async get(key) { + console.log('cache-handler get', key) + return cache.get(key) + } + + async set(key, data) { + console.log('cache-handler set', key) + cache.set(key, { + value: data, + lastModified: Date.now(), + }) + } +} diff --git a/test/e2e/app-dir/app-static/next.config.js b/test/e2e/app-dir/app-static/next.config.js index f33487a87e..d3dcb8b14b 100644 --- a/test/e2e/app-dir/app-static/next.config.js +++ b/test/e2e/app-dir/app-static/next.config.js @@ -1,6 +1,9 @@ module.exports = { experimental: { appDir: true, + incrementalCacheHandlerPath: process.env.CUSTOM_CACHE_HANDLER + ? require.resolve('./cache-handler.js') + : undefined, }, // assetPrefix: '/assets', rewrites: async () => {