From 44cdb9f5deda74acbe3f8c60f3c7321718f2c4b0 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 5 Feb 2025 19:41:11 +0000 Subject: [PATCH 1/2] allow `null` to be returned as an incremental cache get result --- packages/open-next/src/adapters/cache.ts | 21 ++++++++----------- .../src/core/routing/cacheInterceptor.ts | 4 ++-- .../incrementalCache/multi-tier-ddb-s3.ts | 2 +- packages/open-next/src/types/overrides.ts | 2 +- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index 7b7da8632..d19c72016 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -130,21 +130,18 @@ export default class Cache { async getFetchCache(key: string, softTags?: string[], tags?: string[]) { debug("get fetch cache", { key, softTags, tags }); try { - const { value, lastModified } = await globalThis.incrementalCache.get( - key, - true, - ); + const cachedEntry = await globalThis.incrementalCache.get(key, true); const _lastModified = await globalThis.tagCache.getLastModified( key, - lastModified, + cachedEntry?.lastModified, ); if (_lastModified === -1) { // If some tags are stale we need to force revalidation return null; } - if (value === undefined) return null; + if (cachedEntry?.value === undefined) return null; // For cases where we don't have tags, we need to ensure that the soft tags are not being revalidated // We only need to check for the path as it should already contain all the tags @@ -159,7 +156,7 @@ export default class Cache { if (path) { const pathLastModified = await globalThis.tagCache.getLastModified( path.replace("_N_T_/", ""), - lastModified, + cachedEntry.lastModified, ); if (pathLastModified === -1) { // In case the path has been revalidated, we don't want to use the fetch cache @@ -170,7 +167,7 @@ export default class Cache { return { lastModified: _lastModified, - value: value, + value: cachedEntry.value, } as CacheHandlerValue; } catch (e) { // We can usually ignore errors here as they are usually due to cache not being found @@ -181,18 +178,18 @@ export default class Cache { async getIncrementalCache(key: string): Promise { try { - const { value: cacheData, lastModified } = - await globalThis.incrementalCache.get(key, false); + const cachedEntry = await globalThis.incrementalCache.get(key, false); - const meta = cacheData?.meta; + const meta = cachedEntry?.value?.meta; const _lastModified = await globalThis.tagCache.getLastModified( key, - lastModified, + cachedEntry?.lastModified, ); if (_lastModified === -1) { // If some tags are stale we need to force revalidation return null; } + const cacheData = cachedEntry?.value; const requestId = globalThis.__openNextAls.getStore()?.requestId ?? ""; globalThis.lastModified[requestId] = _lastModified; if (cacheData?.type === "route") { diff --git a/packages/open-next/src/core/routing/cacheInterceptor.ts b/packages/open-next/src/core/routing/cacheInterceptor.ts index 932184e72..25786c56b 100644 --- a/packages/open-next/src/core/routing/cacheInterceptor.ts +++ b/packages/open-next/src/core/routing/cacheInterceptor.ts @@ -157,7 +157,7 @@ export async function cacheInterceptor( try { const cachedData = await globalThis.incrementalCache.get(localizedPath); debug("cached data in interceptor", cachedData); - if (cachedData.value?.type === "app") { + if (cachedData?.value?.type === "app") { // We need to check the tag cache now const _lastModified = await globalThis.tagCache.getLastModified( localizedPath, @@ -169,7 +169,7 @@ export async function cacheInterceptor( } } const host = event.headers.host; - switch (cachedData.value?.type) { + switch (cachedData?.value?.type) { case "app": case "page": return generateResult( diff --git a/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts b/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts index bc2e86c46..35f2702a8 100644 --- a/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts +++ b/packages/open-next/src/overrides/incrementalCache/multi-tier-ddb-s3.ts @@ -92,7 +92,7 @@ const multiTierCache: IncrementalCache = { } } const result = await S3Cache.get(key, isFetch); - if (result.value) { + if (result?.value) { localCache.set(key, { value: result.value, lastModified: result.lastModified ?? Date.now(), diff --git a/packages/open-next/src/types/overrides.ts b/packages/open-next/src/types/overrides.ts index 00e81a60e..4e87fc6a4 100644 --- a/packages/open-next/src/types/overrides.ts +++ b/packages/open-next/src/types/overrides.ts @@ -69,7 +69,7 @@ export type IncrementalCache = { get( key: string, isFetch?: IsFetch, - ): Promise>>; + ): Promise> | null>; set( key: string, value: CacheValue, From e8961abcff80e5b39eb9e3d4025215a456efc79a Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 5 Feb 2025 21:07:00 +0000 Subject: [PATCH 2/2] add early returns as suggested in PR --- packages/open-next/src/adapters/cache.ts | 10 +++++++--- .../open-next/src/core/routing/cacheInterceptor.ts | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts index d19c72016..4de87abc5 100644 --- a/packages/open-next/src/adapters/cache.ts +++ b/packages/open-next/src/adapters/cache.ts @@ -132,6 +132,8 @@ export default class Cache { try { const cachedEntry = await globalThis.incrementalCache.get(key, true); + if (cachedEntry?.value === undefined) return null; + const _lastModified = await globalThis.tagCache.getLastModified( key, cachedEntry?.lastModified, @@ -141,8 +143,6 @@ export default class Cache { return null; } - if (cachedEntry?.value === undefined) return null; - // For cases where we don't have tags, we need to ensure that the soft tags are not being revalidated // We only need to check for the path as it should already contain all the tags if ((tags ?? []).length === 0) { @@ -180,7 +180,11 @@ export default class Cache { try { const cachedEntry = await globalThis.incrementalCache.get(key, false); - const meta = cachedEntry?.value?.meta; + if (!cachedEntry?.value) { + return null; + } + + const meta = cachedEntry.value.meta; const _lastModified = await globalThis.tagCache.getLastModified( key, cachedEntry?.lastModified, diff --git a/packages/open-next/src/core/routing/cacheInterceptor.ts b/packages/open-next/src/core/routing/cacheInterceptor.ts index 25786c56b..1e387cf67 100644 --- a/packages/open-next/src/core/routing/cacheInterceptor.ts +++ b/packages/open-next/src/core/routing/cacheInterceptor.ts @@ -157,6 +157,11 @@ export async function cacheInterceptor( try { const cachedData = await globalThis.incrementalCache.get(localizedPath); debug("cached data in interceptor", cachedData); + + if (!cachedData?.value) { + return event; + } + if (cachedData?.value?.type === "app") { // We need to check the tag cache now const _lastModified = await globalThis.tagCache.getLastModified(