refactor: lastModified moved to ALS

opennextjs · Feb 12, 2025 · e079a59 · e079a59
1 parent 784958f
commit e079a59
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 36 deletions.
diff --git a/.changeset/healthy-elephants-scream.md b/.changeset/healthy-elephants-scream.md
@@ -0,0 +1,7 @@
+---
+"@opennextjs/aws": minor
+---
+
+refactor: lastModified moved to ALS
+
+BREAKING CHANGE: `lastModified` is moved to ALS as a number from a global map indexed by `requestId`
diff --git a/packages/open-next/src/adapters/cache.ts b/packages/open-next/src/adapters/cache.ts
@@ -194,8 +194,10 @@ export default class Cache {
         return null;
       }
       const cacheData = cachedEntry?.value;
-      const requestId = globalThis.__openNextAls.getStore()?.requestId ?? "";
-      globalThis.lastModified[requestId] = _lastModified;
+      const store = globalThis.__openNextAls.getStore();
+      if (store) {
+        store.lastModified = _lastModified;
+      }
       if (cacheData?.type === "route") {
         return {
           lastModified: _lastModified,

diff --git a/packages/open-next/src/core/createMainHandler.ts b/packages/open-next/src/core/createMainHandler.ts
@@ -43,8 +43,6 @@ export async function createMainHandler() {
     thisFunction.override?.cdnInvalidation,
   );
 
-  globalThis.lastModified = {};
-
   // From the config, we create the converter
   const converter = await resolveConverter(thisFunction.override?.converter);
 

diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts
@@ -193,12 +193,6 @@ export async function openNextHandler(
         body,
         isBase64Encoded,
       };
-      const requestId = store?.requestId;
-
-      if (requestId) {
-        // reset lastModified. We need to do this to avoid memory leaks
-        delete globalThis.lastModified[requestId];
-      }
 
       return internalResult;
     },

diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts
@@ -289,12 +289,10 @@ export async function revalidateIfRequired(
     try {
       const hash = (str: string) =>
         crypto.createHash("md5").update(str).digest("hex");
-      const requestId = globalThis.__openNextAls.getStore()?.requestId ?? "";
 
-      const lastModified =
-        globalThis.lastModified[requestId] > 0
-          ? globalThis.lastModified[requestId]
-          : "";
+      const lastModified = String(
+        globalThis.__openNextAls.getStore()?.lastModified ?? "",
+      );
 
       // For some weird cases, lastModified is not set, haven't been able to figure out yet why
       // For those cases we add the etag to the deduplication id, it might help
@@ -321,15 +319,14 @@ export function fixISRHeaders(headers: OutgoingHttpHeaders) {
       "private, no-cache, no-store, max-age=0, must-revalidate";
     return;
   }
-  const requestId = globalThis.__openNextAls.getStore()?.requestId ?? "";
-  const _lastModified = globalThis.lastModified[requestId] ?? 0;
+  const _lastModified = globalThis.__openNextAls.getStore()?.lastModified ?? 0;
   if (headers[CommonHeaders.NEXT_CACHE] === "HIT" && _lastModified > 0) {
     // calculate age
     const age = Math.round((Date.now() - _lastModified) / 1000);
     // extract s-maxage from cache-control
     const regex = /s-maxage=(\d+)/;
     const cacheControl = headers[CommonHeaders.CACHE_CONTROL];
-    debug("cache-control", cacheControl, globalThis.lastModified, Date.now());
+    debug("cache-control", cacheControl, _lastModified, Date.now());
     if (typeof cacheControl !== "string") return;
     const match = cacheControl.match(regex);
     const sMaxAge = match ? Number.parseInt(match[1]) : undefined;

diff --git a/packages/open-next/src/types/global.ts b/packages/open-next/src/types/global.ts
@@ -55,10 +55,13 @@ export interface EdgeRoute {
 }
 
 interface OpenNextRequestContext {
+  // Unique ID for the request.
   requestId: string;
   pendingPromiseRunner: DetachedPromiseRunner;
   isISRRevalidation?: boolean;
   mergeHeadersPriority?: "middleware" | "handler";
+  // Last modified time of the page (used in main functions).
+  lastModified?: number;
   waitUntil?: WaitUntil;
 }
 
@@ -99,14 +102,6 @@ declare global {
    */
   var disableIncrementalCache: boolean;
 
-  /**
-   * An object that contains the last modified time of the pages.
-   * Only available in main functions.
-   * TODO: Integrate this directly in the AsyncLocalStorage context
-   * Defined in `createMainHandler`.
-   */
-  var lastModified: Record<string, number>;
-
   /**
    * A boolean that indicates if Next is V15 or higher.
    * Only available in the cache adapter.

diff --git a/packages/tests-unit/tests/adapters/cache.test.ts b/packages/tests-unit/tests/adapters/cache.test.ts
@@ -49,7 +49,6 @@ describe("CacheHandler", () => {
 
   globalThis.__openNextAls = {
     getStore: vi.fn().mockReturnValue({
-      requestId: "123",
       pendingPromiseRunner: {
         withResolvers: vi.fn().mockReturnValue({
           resolve: vi.fn(),
@@ -69,8 +68,6 @@ describe("CacheHandler", () => {
       },
     };
     globalThis.isNextAfter15 = false;
-
-    globalThis.lastModified = {};
   });
 
   describe("get", () => {

diff --git a/packages/tests-unit/tests/core/routing/util.test.ts b/packages/tests-unit/tests/core/routing/util.test.ts
@@ -648,8 +648,6 @@ describe("revalidateIfRequired", () => {
     globalThis.__openNextAls = {
       getStore: vi.fn(),
     };
-
-    globalThis.lastModified = {};
   });
 
   it("should not send to queue when x-nextjs-cache is not present", async () => {
@@ -692,13 +690,9 @@ describe("fixISRHeaders", () => {
     vi.useFakeTimers().setSystemTime("2024-01-02T00:00:00Z");
     globalThis.__openNextAls = {
       getStore: () => ({
-        requestId: "123",
+        lastModified: new Date("2024-01-01T12:00:00Z").getTime(),
       }),
     };
-
-    globalThis.lastModified = {
-      "123": new Date("2024-01-01T12:00:00Z").getTime(),
-    };
   });
 
   it("should set cache-control directive to must-revalidate when x-nextjs-cache is REVALIDATED", () => {