From 83f685e15552cee1c8a4d6c68251c01ff1cfe3fb Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Thu, 29 Aug 2024 20:38:43 +0200 Subject: [PATCH] fix: update cache handler to accommodate changes in next@canary (#2572) * update react version needed by next@canary * tmp: just checking if canary tests will be happier * what's up with NODE_ENV? * missing PAGES * chore: use npm info to figure out react version needed for canary instead of hardcoding it * chore: drop package lock file when preparing fixtures * test: unset more things related to next's fetch patching * use correct cache kind for initial cache seeding depending on next version * small cleanup * typehell * just checking * proper NODE_ENV setting with explanation * any is bad * Update src/shared/cache-types.cts Co-authored-by: Philippe Serhal * add note about deleting next-patch symbol --------- Co-authored-by: Philippe Serhal --- src/build/content/prerendered.ts | 31 +++++-- src/run/handlers/cache.cts | 67 +++++++++------ src/run/next.cts | 9 ++ src/shared/cache-types.cts | 122 +++++++++++++++++++++------ tests/prepare.mjs | 7 +- tests/utils/fixture.ts | 4 + tests/utils/next-version-helpers.mjs | 14 ++- 7 files changed, 194 insertions(+), 60 deletions(-) diff --git a/src/build/content/prerendered.ts b/src/build/content/prerendered.ts index 9f1ae2fe75..6c45a74472 100644 --- a/src/build/content/prerendered.ts +++ b/src/build/content/prerendered.ts @@ -10,7 +10,7 @@ import { satisfies } from 'semver' import { encodeBlobKey } from '../../shared/blobkey.js' import type { - CachedFetchValue, + CachedFetchValueForMultipleVersions, NetlifyCachedAppPageValue, NetlifyCachedPageValue, NetlifyCachedRouteValue, @@ -45,8 +45,11 @@ const writeCacheEntry = async ( */ const routeToFilePath = (path: string) => (path === '/' ? '/index' : path) -const buildPagesCacheValue = async (path: string): Promise => ({ - kind: 'PAGE', +const buildPagesCacheValue = async ( + path: string, + shouldUseEnumKind: boolean, +): Promise => ({ + kind: shouldUseEnumKind ? 'PAGES' : 'PAGE', html: await readFile(`${path}.html`, 'utf-8'), pageData: JSON.parse(await readFile(`${path}.json`, 'utf-8')), headers: undefined, @@ -96,14 +99,17 @@ const buildAppCacheValue = async ( const buildRouteCacheValue = async ( path: string, initialRevalidateSeconds: number | false, + shouldUseEnumKind: boolean, ): Promise => ({ - kind: 'ROUTE', + kind: shouldUseEnumKind ? 'APP_ROUTE' : 'ROUTE', body: await readFile(`${path}.body`, 'base64'), ...JSON.parse(await readFile(`${path}.meta`, 'utf-8')), revalidate: initialRevalidateSeconds, }) -const buildFetchCacheValue = async (path: string): Promise => ({ +const buildFetchCacheValue = async ( + path: string, +): Promise => ({ kind: 'FETCH', ...JSON.parse(await readFile(path, 'utf-8')), }) @@ -133,6 +139,13 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise }) : false + // https://github.com/vercel/next.js/pull/68602 changed the cache kind for Pages router pages from `PAGE` to `PAGES` and from `ROUTE` to `APP_ROUTE`. + const shouldUseEnumKind = ctx.nextVersion + ? satisfies(ctx.nextVersion, '>=15.0.0-canary.114 <15.0.0-d || >15.0.0-rc.0', { + includePrerelease: true, + }) + : false + await Promise.all( Object.entries(manifest.routes).map( ([route, meta]): Promise => @@ -152,7 +165,10 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise // if pages router returns 'notFound: true', build won't produce html and json files return } - value = await buildPagesCacheValue(join(ctx.publishDir, 'server/pages', key)) + value = await buildPagesCacheValue( + join(ctx.publishDir, 'server/pages', key), + shouldUseEnumKind, + ) break case meta.dataRoute?.endsWith('.rsc'): value = await buildAppCacheValue( @@ -164,6 +180,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise value = await buildRouteCacheValue( join(ctx.publishDir, 'server/app', key), meta.initialRevalidateSeconds, + shouldUseEnumKind, ) break default: @@ -171,7 +188,7 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise } // Netlify Forms are not support and require a workaround - if (value.kind === 'PAGE' || value.kind === 'APP_PAGE') { + if (value.kind === 'PAGE' || value.kind === 'PAGES' || value.kind === 'APP_PAGE') { verifyNetlifyForms(ctx, value.html) } diff --git a/src/run/handlers/cache.cts b/src/run/handlers/cache.cts index 790c89004e..32e569c065 100644 --- a/src/run/handlers/cache.cts +++ b/src/run/handlers/cache.cts @@ -11,14 +11,15 @@ import { type Span } from '@opentelemetry/api' import type { PrerenderManifest } from 'next/dist/build/index.js' import { NEXT_CACHE_TAGS_HEADER } from 'next/dist/lib/constants.js' -import type { - CacheHandler, - CacheHandlerContext, - IncrementalCache, - NetlifyCachedPageValue, - NetlifyCachedRouteValue, - NetlifyCacheHandlerValue, - NetlifyIncrementalCacheValue, +import { + type CacheHandlerContext, + type CacheHandlerForMultipleVersions, + isCachedPageValue, + isCachedRouteValue, + type NetlifyCachedPageValue, + type NetlifyCachedRouteValue, + type NetlifyCacheHandlerValue, + type NetlifyIncrementalCacheValue, } from '../../shared/cache-types.cjs' import { getRegionalBlobStore } from '../regional-blob-store.cjs' @@ -29,7 +30,7 @@ type TagManifest = { revalidatedAt: number } type TagManifestBlobCache = Record> -export class NetlifyCacheHandler implements CacheHandler { +export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions { options: CacheHandlerContext revalidatedTags: string[] blobStore: Store @@ -132,13 +133,18 @@ export class NetlifyCacheHandler implements CacheHandler { if ( cacheValue.kind === 'PAGE' || + cacheValue.kind === 'PAGES' || cacheValue.kind === 'APP_PAGE' || - cacheValue.kind === 'ROUTE' + cacheValue.kind === 'ROUTE' || + cacheValue.kind === 'APP_ROUTE' ) { if (cacheValue.headers?.[NEXT_CACHE_TAGS_HEADER]) { const cacheTags = (cacheValue.headers[NEXT_CACHE_TAGS_HEADER] as string).split(',') requestContext.responseCacheTags = cacheTags - } else if (cacheValue.kind === 'PAGE' && typeof cacheValue.pageData === 'object') { + } else if ( + (cacheValue.kind === 'PAGE' || cacheValue.kind === 'PAGES') && + typeof cacheValue.pageData === 'object' + ) { // pages router doesn't have cache tags headers in PAGE cache value // so we need to generate appropriate cache tags for it const cacheTags = [`_N_T_${key === '/index' ? '/' : key}`] @@ -185,7 +191,9 @@ export class NetlifyCacheHandler implements CacheHandler { } } - async get(...args: Parameters): ReturnType { + async get( + ...args: Parameters + ): ReturnType { return this.tracer.withActiveSpan('get cache key', async (span) => { const [key, ctx = {}] = args getLogger().debug(`[NetlifyCacheHandler.get]: ${key}`) @@ -224,8 +232,12 @@ export class NetlifyCacheHandler implements CacheHandler { value: blob.value, } - case 'ROUTE': { - span.addEvent('ROUTE', { lastModified: blob.lastModified, status: blob.value.status }) + case 'ROUTE': + case 'APP_ROUTE': { + span.addEvent(blob.value?.kind, { + lastModified: blob.lastModified, + status: blob.value.status, + }) const valueWithoutRevalidate = this.captureRouteRevalidateAndRemoveFromObject(blob.value) @@ -237,8 +249,9 @@ export class NetlifyCacheHandler implements CacheHandler { }, } } - case 'PAGE': { - span.addEvent('PAGE', { lastModified: blob.lastModified }) + case 'PAGE': + case 'PAGES': { + span.addEvent(blob.value?.kind, { lastModified: blob.lastModified }) const { revalidate, ...restOfPageValue } = blob.value @@ -250,7 +263,7 @@ export class NetlifyCacheHandler implements CacheHandler { } } case 'APP_PAGE': { - span.addEvent('APP_PAGE', { lastModified: blob.lastModified }) + span.addEvent(blob.value?.kind, { lastModified: blob.lastModified }) const { revalidate, rscData, ...restOfPageValue } = blob.value @@ -272,10 +285,14 @@ export class NetlifyCacheHandler implements CacheHandler { } private transformToStorableObject( - data: Parameters[1], - context: Parameters[2], + data: Parameters[1], + context: Parameters[2], ): NetlifyIncrementalCacheValue | null { - if (data?.kind === 'ROUTE') { + if (!data) { + return null + } + + if (isCachedRouteValue(data)) { return { ...data, revalidate: context.revalidate, @@ -283,7 +300,7 @@ export class NetlifyCacheHandler implements CacheHandler { } } - if (data?.kind === 'PAGE') { + if (isCachedPageValue(data)) { return { ...data, revalidate: context.revalidate, @@ -301,7 +318,7 @@ export class NetlifyCacheHandler implements CacheHandler { return data } - async set(...args: Parameters) { + async set(...args: Parameters) { return this.tracer.withActiveSpan('set cache key', async (span) => { const [key, data, context] = args const blobKey = await this.encodeBlobKey(key) @@ -321,7 +338,7 @@ export class NetlifyCacheHandler implements CacheHandler { value, }) - if (data?.kind === 'PAGE') { + if (data?.kind === 'PAGE' || data?.kind === 'PAGES') { const requestContext = getRequestContext() if (requestContext?.didPagesRouterOnDemandRevalidate) { const tag = `_N_T_${key === '/index' ? '/' : key}` @@ -397,8 +414,10 @@ export class NetlifyCacheHandler implements CacheHandler { cacheTags = [...tags, ...softTags] } else if ( cacheEntry.value?.kind === 'PAGE' || + cacheEntry.value?.kind === 'PAGES' || cacheEntry.value?.kind === 'APP_PAGE' || - cacheEntry.value?.kind === 'ROUTE' + cacheEntry.value?.kind === 'ROUTE' || + cacheEntry.value?.kind === 'APP_ROUTE' ) { cacheTags = (cacheEntry.value.headers?.[NEXT_CACHE_TAGS_HEADER] as string)?.split(',') || [] } else { diff --git a/src/run/next.cts b/src/run/next.cts index edb6fd2a44..b99515e393 100644 --- a/src/run/next.cts +++ b/src/run/next.cts @@ -8,6 +8,15 @@ import { getRequestContext } from './handlers/request-context.cjs' import { getTracer } from './handlers/tracer.cjs' import { getRegionalBlobStore } from './regional-blob-store.cjs' +// https://github.com/vercel/next.js/pull/68193/files#diff-37243d614f1f5d3f7ea50bbf2af263f6b1a9a4f70e84427977781e07b02f57f1R49 +// This import resulted in importing unbundled React which depending if NODE_ENV is `production` or not would use +// either development or production version of React. When not set to `production` it would use development version +// which later cause mismatching problems when both development and production versions of React were loaded causing +// react errors. +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore ignoring readonly NODE_ENV +process.env.NODE_ENV = 'production' + console.time('import next server') // eslint-disable-next-line @typescript-eslint/no-var-requires diff --git a/src/shared/cache-types.cts b/src/shared/cache-types.cts index 06cee07516..bdc48bb973 100644 --- a/src/shared/cache-types.cts +++ b/src/shared/cache-types.cts @@ -1,53 +1,78 @@ import type { + CacheHandler, CacheHandlerValue, - IncrementalCache, } from 'next/dist/server/lib/incremental-cache/index.js' import type { + CachedFetchValue, CachedRouteValue, IncrementalCachedAppPageValue, + IncrementalCachedPageValue, IncrementalCacheValue, } from 'next/dist/server/response-cache/types.js' -export type { - CacheHandler, - CacheHandlerContext, - IncrementalCache, -} from 'next/dist/server/lib/incremental-cache/index.js' +export type { CacheHandlerContext } from 'next/dist/server/lib/incremental-cache/index.js' -export type NetlifyCachedRouteValue = Omit & { +/** + * Shape of the cache value that is returned from CacheHandler.get or passed to CacheHandler.set + */ +type CachedRouteValueForMultipleVersions = Omit & { + kind: 'ROUTE' | 'APP_ROUTE' +} + +/** + * Used for storing in blobs and reading from blobs + */ +export type NetlifyCachedRouteValue = Omit & { // Next.js stores body as buffer, while we store it as base64 encoded string body: string // Next.js doesn't produce cache-control tag we use to generate cdn cache control // so store needed values as part of cached response data - revalidate: Parameters[2]['revalidate'] + revalidate?: Parameters[2]['revalidate'] } -export type NetlifyCachedAppPageValue = Omit & { +/** + * Shape of the cache value that is returned from CacheHandler.get or passed to CacheHandler.set + */ +type IncrementalCachedAppPageValueForMultipleVersions = Omit< + IncrementalCachedAppPageValue, + 'kind' +> & { + kind: 'APP_PAGE' +} + +/** + * Used for storing in blobs and reading from blobs + */ +export type NetlifyCachedAppPageValue = Omit< + IncrementalCachedAppPageValueForMultipleVersions, + 'rscData' +> & { // Next.js stores rscData as buffer, while we store it as base64 encoded string rscData: string | undefined - revalidate?: Parameters[2]['revalidate'] + revalidate?: Parameters[2]['revalidate'] } -type CachedPageValue = Extract - -export type NetlifyCachedPageValue = CachedPageValue & { - revalidate?: Parameters[2]['revalidate'] +/** + * Shape of the cache value that is returned from CacheHandler.get or passed to CacheHandler.set + */ +type IncrementalCachedPageValueForMultipleVersions = Omit & { + kind: 'PAGE' | 'PAGES' } -export type CachedFetchValue = Extract +/** + * Used for storing in blobs and reading from blobs + */ +export type NetlifyCachedPageValue = IncrementalCachedPageValueForMultipleVersions & { + revalidate?: Parameters[2]['revalidate'] +} -export type NetlifyIncrementalCacheValue = - | Exclude< - IncrementalCacheValue, - CachedRouteValue | CachedPageValue | IncrementalCachedAppPageValue - > - | NetlifyCachedRouteValue - | NetlifyCachedPageValue - | NetlifyCachedAppPageValue +export type CachedFetchValueForMultipleVersions = Omit & { + kind: 'FETCH' +} type CachedRouteValueToNetlify = T extends CachedRouteValue ? NetlifyCachedRouteValue - : T extends CachedPageValue + : T extends IncrementalCachedPageValue ? NetlifyCachedPageValue : T extends IncrementalCachedAppPageValue ? NetlifyCachedAppPageValue @@ -55,4 +80,53 @@ type CachedRouteValueToNetlify = T extends CachedRouteValue type MapCachedRouteValueToNetlify = { [K in keyof T]: CachedRouteValueToNetlify } +/** + * Used for storing in blobs and reading from blobs + */ export type NetlifyCacheHandlerValue = MapCachedRouteValueToNetlify + +/** + * Used for storing in blobs and reading from blobs + */ +export type NetlifyIncrementalCacheValue = NetlifyCacheHandlerValue['value'] + +type IncrementalCacheValueToMultipleVersions = T extends CachedRouteValue + ? CachedRouteValueForMultipleVersions + : T extends IncrementalCachedPageValue + ? IncrementalCachedPageValueForMultipleVersions + : T extends IncrementalCachedAppPageValue + ? IncrementalCachedAppPageValueForMultipleVersions + : T extends CachedFetchValue + ? CachedFetchValueForMultipleVersions + : T extends CacheHandlerValue + ? { + [K in keyof T]: IncrementalCacheValueToMultipleVersions + } + : T + +type IncrementalCacheValueForMultipleVersions = + IncrementalCacheValueToMultipleVersions + +export const isCachedPageValue = ( + value: IncrementalCacheValueForMultipleVersions, +): value is IncrementalCachedPageValueForMultipleVersions => + value.kind === 'PAGE' || value.kind === 'PAGES' + +export const isCachedRouteValue = ( + value: IncrementalCacheValueForMultipleVersions, +): value is CachedRouteValueForMultipleVersions => + value.kind === 'ROUTE' || value.kind === 'APP_ROUTE' + +type MapArgsOrReturn = T extends readonly unknown[] + ? { [K in keyof T]: MapArgsOrReturn } + : T extends Promise + ? Promise> + : IncrementalCacheValueToMultipleVersions + +type MapCacheHandlerClassMethod = T extends (...args: infer Args) => infer Ret + ? (...args: MapArgsOrReturn) => MapArgsOrReturn + : T + +type MapCacheHandlerClass = { [K in keyof T]: MapCacheHandlerClassMethod } + +export type CacheHandlerForMultipleVersions = MapCacheHandlerClass diff --git a/tests/prepare.mjs b/tests/prepare.mjs index 13beeaca2a..5bc35c6f1f 100644 --- a/tests/prepare.mjs +++ b/tests/prepare.mjs @@ -44,14 +44,17 @@ const promises = fixtures.map((fixture) => }) } - // npm is the default - let cmd = `npm install --no-audit --progress=false --prefer-offline --legacy-peer-deps` + let cmd = `` const { packageManager } = JSON.parse(await readFile(join(cwd, 'package.json'), 'utf8')) if (packageManager?.startsWith('pnpm')) { // We disable frozen-lockfile because we may have changed the version of Next.js cmd = `pnpm install --frozen-lockfile=false ${ process.env.DEBUG || NEXT_VERSION !== 'latest' ? '' : '--reporter silent' }` + } else { + // npm is the default + cmd = `npm install --no-audit --progress=false --prefer-offline --legacy-peer-deps` + await rm(join(cwd, 'package-lock.json'), { force: true }) } const addPrefix = new Transform({ diff --git a/tests/utils/fixture.ts b/tests/utils/fixture.ts index 18ebb328fd..db86b9ea32 100644 --- a/tests/utils/fixture.ts +++ b/tests/utils/fixture.ts @@ -89,6 +89,10 @@ export const createFixture = async (fixture: string, ctx: FixtureTestContext) => ) { // @ts-ignore fetch doesn't have _nextOriginalFetch property in types globalThis.fetch = globalThis._nextOriginalFetch || globalThis.fetch._nextOriginalFetch + // https://github.com/vercel/next.js/pull/68193/files#diff-4c54e369ddb9a2db1eed95fe1d678f94c8e82c540204475d42c78e49bf4f223aR37-R40 + // above changed the way Next.js checks wether fetch was already patched. It still sets `__nextPatched` and `_nextOriginalFetch` + // properties we check above and use to get original fetch back + delete globalThis[Symbol.for('next-patch')] } ctx.cwd = await mkdtemp(join(tmpdir(), 'netlify-next-runtime-')) diff --git a/tests/utils/next-version-helpers.mjs b/tests/utils/next-version-helpers.mjs index abf4053471..f9b158e85a 100644 --- a/tests/utils/next-version-helpers.mjs +++ b/tests/utils/next-version-helpers.mjs @@ -4,12 +4,11 @@ import { readFile, writeFile } from 'node:fs/promises' import fg from 'fast-glob' import { gte, satisfies, valid } from 'semver' +import { execaCommand } from 'execa' const FUTURE_NEXT_PATCH_VERSION = '14.999.0' const NEXT_VERSION_REQUIRES_REACT_19 = '14.3.0-canary.45' -// TODO: Update this when React 19 is released -const REACT_19_VERSION = '19.0.0-rc.0' const REACT_18_VERSION = '18.2.0' /** @@ -97,8 +96,17 @@ export async function setNextVersionInFixture( return } packageJson.dependencies.next = version + + const { stdout } = await execaCommand( + `npm info next@${resolvedVersion} peerDependencies --json`, + { cwd }, + ) + + const nextPeerDependencies = JSON.parse(stdout) + if (updateReact && nextVersionRequiresReact19(checkVersion)) { - const reactVersion = operation === 'update' ? REACT_19_VERSION : REACT_18_VERSION + const reactVersion = + operation === 'update' ? nextPeerDependencies['react'] : REACT_18_VERSION packageJson.dependencies.react = reactVersion packageJson.dependencies['react-dom'] = reactVersion }