refactor: code simplification (#627)

* refactor: code simplification * GitButler WIP Commit fixup! biome * fixup! tests * fixup: !format * fixup! (thank you tests) * fixup! I'll get there
opennextjs · Nov 11, 2024 · 08874fb · 08874fb
1 parent 04379e2
commit 08874fb
Show file tree

Hide file tree

Showing 40 changed files with 250 additions and 235 deletions.
diff --git a/packages/open-next/package.json b/packages/open-next/package.json
@@ -47,7 +47,7 @@
     "@types/aws-lambda": "^8.10.109",
     "@types/node": "^18.16.1",
     "tsc-alias": "^1.8.8",
-    "typescript": "^4.9.3"
+    "typescript": "^5.6.3"
   },
   "bugs": {
     "url": "https://github.com/opennextjs/opennextjs-aws/issues"

diff --git a/packages/open-next/src/adapters/dynamo-provider.ts b/packages/open-next/src/adapters/dynamo-provider.ts
@@ -56,7 +56,7 @@ async function insert(
   const parsedData = data.map((item) => ({
     tag: item.tag.S,
     path: item.path.S,
-    revalidatedAt: parseInt(item.revalidatedAt.N),
+    revalidatedAt: Number.parseInt(item.revalidatedAt.N),
   }));
 
   await tagCache.writeTags(parsedData);

diff --git a/packages/open-next/src/adapters/server-adapter.ts b/packages/open-next/src/adapters/server-adapter.ts
@@ -28,7 +28,8 @@ export const handler = await createMainHandler();
 //////////////////////
 
 function setNextjsServerWorkingDirectory() {
-  // WORKAROUND: Set `NextServer` working directory (AWS specific) — https://github.com/serverless-stack/open-next#workaround-set-nextserver-working-directory-aws-specific
+  // WORKAROUND: Set `NextServer` working directory (AWS specific)
+  // See https://opennext.js.org/aws/v2/advanced/workaround#workaround-set-nextserver-working-directory-aws-specific
   process.chdir(__dirname);
 }
 

diff --git a/packages/open-next/src/adapters/util.ts b/packages/open-next/src/adapters/util.ts
@@ -31,7 +31,7 @@ export function parseNumberFromEnv(
     return envValue;
   }
 
-  const parsedValue = parseInt(envValue);
+  const parsedValue = Number.parseInt(envValue);
 
   return isNaN(parsedValue) ? undefined : parsedValue;
 }
diff --git a/packages/open-next/src/build/createAssets.ts b/packages/open-next/src/build/createAssets.ts
@@ -14,12 +14,17 @@ export function createStaticAssets(options: buildHelper.BuildOptions) {
   const outputPath = path.join(outputDir, "assets");
   fs.mkdirSync(outputPath, { recursive: true });
 
-  // Next.js outputs assets into multiple files. Copy into the same directory.
-  // Copy over:
-  // - .next/BUILD_ID => _next/BUILD_ID
-  // - .next/static   => _next/static
-  // - public/*       => *
-  // - app/favicon.ico or src/app/favicon.ico  => favicon.ico
+  /**
+   * Next.js outputs assets into multiple files. Copy into the same directory.
+   *
+   * Copy over:
+   * - .next/BUILD_ID => BUILD_ID
+   * - .next/static   => _next/static
+   * - public/*       => *
+   * - app/favicon.ico or src/app/favicon.ico  => favicon.ico
+   *
+   * Note: BUILD_ID is used by the SST infra.
+   */
   fs.copyFileSync(
     path.join(appBuildOutputPath, ".next/BUILD_ID"),
     path.join(outputPath, "BUILD_ID"),

diff --git a/packages/open-next/src/build/createMiddleware.ts b/packages/open-next/src/build/createMiddleware.ts
@@ -2,9 +2,9 @@ import fs from "node:fs";
 import path from "node:path";
 
 import logger from "../logger.js";
-import {
-  type MiddlewareInfo,
-  type MiddlewareManifest,
+import type {
+  MiddlewareInfo,
+  MiddlewareManifest,
 } from "../types/next-types.js";
 import { buildEdgeBundle } from "./edge/createEdgeBundle.js";
 import * as buildHelper from "./helper.js";

diff --git a/packages/open-next/src/build/createServerBundle.ts b/packages/open-next/src/build/createServerBundle.ts
@@ -197,25 +197,18 @@ async function generateBundle(
         ...(disableNextPrebundledReact ? ["requireHooks"] : []),
         ...(disableRouting ? ["trustHostHeader"] : []),
         ...(!isBefore13413 ? ["requestHandlerHost"] : []),
-        ...(!isAfter141 ? ["stableIncrementalCache"] : []),
-        ...(isAfter141 ? ["experimentalIncrementalCacheHandler"] : []),
+        ...(isAfter141
+          ? ["experimentalIncrementalCacheHandler"]
+          : ["stableIncrementalCache"]),
       ],
     }),
 
     openNextResolvePlugin({
       fnName: name,
-      overrides: overrides,
+      overrides,
     }),
   ];
 
-  if (plugins && plugins.length > 0) {
-    logger.debug(
-      `Applying plugins:: [${plugins
-        .map(({ name }) => name)
-        .join(",")}] for Next version: ${options.nextVersion}`,
-    );
-  }
-
   const outfileExt = fnOptions.runtime === "deno" ? "ts" : "mjs";
   await buildHelper.esbuildAsync(
     {
@@ -238,9 +231,12 @@ async function generateBundle(
       },
       plugins,
       alias: {
-        "next/dist/server/next-server.js": isBundled
-          ? "./next-server.runtime.prod.js"
-          : "next/dist/server/next-server.js",
+        ...(isBundled
+          ? {
+              "next/dist/server/next-server.js":
+                "./next-server.runtime.prod.js",
+            }
+          : {}),
       },
     },
     options,

diff --git a/packages/open-next/src/build/edge/createEdgeBundle.ts b/packages/open-next/src/build/edge/createEdgeBundle.ts
@@ -52,7 +52,6 @@ export async function buildEdgeBundle({
   await esbuildAsync(
     {
       entryPoints: [entrypoint],
-      // inject: ,
       bundle: true,
       outfile,
       external: ["node:*", "next", "@aws-sdk/*"],

diff --git a/packages/open-next/src/build/helper.ts b/packages/open-next/src/build/helper.ts
@@ -189,7 +189,7 @@ export function traverseFiles(
   root: string,
   conditionFn: (paths: TraversePath) => boolean,
   callbackFn: (paths: TraversePath) => void,
-  searchingDir: string = "",
+  searchingDir = "",
 ) {
   fs.readdirSync(path.join(root, searchingDir)).forEach((file) => {
     const relativePath = path.join(searchingDir, file);

diff --git a/packages/open-next/src/core/patchAsyncStorage.ts b/packages/open-next/src/core/patchAsyncStorage.ts
@@ -3,13 +3,13 @@ const mod = require("module");
 const resolveFilename = mod._resolveFilename;
 
 export function patchAsyncStorage() {
-  mod._resolveFilename = function (
+  mod._resolveFilename = ((
     originalResolveFilename: typeof resolveFilename,
     request: string,
     parent: any,
     isMain: boolean,
     options: any,
-  ) {
+  ) => {
     if (
       request.endsWith("static-generation-async-storage.external") ||
       request.endsWith("static-generation-async-storage.external.js")
@@ -35,5 +35,5 @@ export function patchAsyncStorage() {
       );
 
     // We use `bind` here to avoid referencing outside variables to create potential memory leaks.
-  }.bind(null, resolveFilename);
+  }).bind(null, resolveFilename);
 }
diff --git a/packages/open-next/src/core/requestHandler.ts b/packages/open-next/src/core/requestHandler.ts
@@ -9,7 +9,10 @@ import { debug, error, warn } from "../adapters/logger";
 import { patchAsyncStorage } from "./patchAsyncStorage";
 import { convertRes, createServerResponse, proxyRequest } from "./routing/util";
 import type { MiddlewareOutputEvent } from "./routingHandler";
-import routingHandler from "./routingHandler";
+import routingHandler, {
+  MIDDLEWARE_HEADER_PREFIX,
+  MIDDLEWARE_HEADER_PREFIX_LEN,
+} from "./routingHandler";
 import { requestHandler, setNextjsPrebundledReact } from "./util";
 
 // This is used to identify requests in the cache
@@ -51,107 +54,103 @@ export async function openNextHandler(
       ? preprocessResult.headers
       : preprocessResult.internalEvent.headers;
 
-  const overwrittenResponseHeaders = Object.entries(
-    "type" in preprocessResult
-      ? preprocessResult.headers
-      : preprocessResult.internalEvent.headers,
-  ).reduce((acc, [key, value]) => {
-    if (!key.startsWith("x-middleware-response-")) {
-      return acc;
-    } else {
-      const newKey = key.replace("x-middleware-response-", "");
-      delete headers[key];
-      headers[newKey] = value;
-      return { ...acc, [newKey]: value };
+  const overwrittenResponseHeaders: Record<string, string | string[]> = {};
+
+  for (const [rawKey, value] of Object.entries(headers)) {
+    if (!rawKey.startsWith(MIDDLEWARE_HEADER_PREFIX)) {
+      continue;
     }
-  }, {});
+    const key = rawKey.slice(MIDDLEWARE_HEADER_PREFIX_LEN);
+    overwrittenResponseHeaders[key] = value;
+    headers[key] = value;
+    delete headers[rawKey];
+  }
 
   if ("type" in preprocessResult) {
-    // // res is used only in the streaming case
+    // response is used only in the streaming case
     if (responseStreaming) {
-      const res = createServerResponse(
+      const response = createServerResponse(
         internalEvent,
         headers,
         responseStreaming,
       );
-      res.statusCode = preprocessResult.statusCode;
-      res.flushHeaders();
+      response.statusCode = preprocessResult.statusCode;
+      response.flushHeaders();
       const [bodyToConsume, bodyToReturn] = preprocessResult.body.tee();
       for await (const chunk of bodyToConsume) {
-        res.write(chunk);
+        response.write(chunk);
       }
-      res.end();
+      response.end();
       preprocessResult.body = bodyToReturn;
     }
     return preprocessResult;
-  } else {
-    const preprocessedEvent = preprocessResult.internalEvent;
-    debug("preprocessedEvent", preprocessedEvent);
-    const reqProps = {
-      method: preprocessedEvent.method,
-      url: preprocessedEvent.url,
-      //WORKAROUND: We pass this header to the serverless function to mimic a prefetch request which will not trigger revalidation since we handle revalidation differently
-      // There is 3 way we can handle revalidation:
-      // 1. We could just let the revalidation go as normal, but due to race condtions the revalidation will be unreliable
-      // 2. We could alter the lastModified time of our cache to make next believe that the cache is fresh, but this could cause issues with stale data since the cdn will cache the stale data as if it was fresh
-      // 3. OUR CHOICE: We could pass a purpose prefetch header to the serverless function to make next believe that the request is a prefetch request and not trigger revalidation (This could potentially break in the future if next changes the behavior of prefetch requests)
-      headers: { ...headers, purpose: "prefetch" },
-      body: preprocessedEvent.body,
-      remoteAddress: preprocessedEvent.remoteAddress,
-    };
-    const requestId = Math.random().toString(36);
-    const pendingPromiseRunner: DetachedPromiseRunner =
-      new DetachedPromiseRunner();
-    const isISRRevalidation = headers["x-isr"] === "1";
-    const mergeHeadersPriority = globalThis.openNextConfig.dangerous
-      ?.headersAndCookiesPriority
-      ? globalThis.openNextConfig.dangerous.headersAndCookiesPriority(
-          preprocessedEvent,
-        )
-      : "middleware";
-    const internalResult = await globalThis.__als.run(
-      {
-        requestId,
-        pendingPromiseRunner,
-        isISRRevalidation,
-        mergeHeadersPriority,
-      },
-      async () => {
-        const preprocessedResult = preprocessResult as MiddlewareOutputEvent;
-        const req = new IncomingMessage(reqProps);
-        const res = createServerResponse(
-          preprocessedEvent,
-          overwrittenResponseHeaders,
-          responseStreaming,
-        );
-
-        await processRequest(
-          req,
-          res,
-          preprocessedEvent,
-          preprocessedResult.isExternalRewrite,
-        );
-
-        const { statusCode, headers, isBase64Encoded, body } = convertRes(res);
-
-        const internalResult = {
-          type: internalEvent.type,
-          statusCode,
-          headers,
-          body,
-          isBase64Encoded,
-        };
-
-        // reset lastModified. We need to do this to avoid memory leaks
-        delete globalThis.lastModified[requestId];
-
-        await pendingPromiseRunner.await();
-
-        return internalResult;
-      },
-    );
-    return internalResult;
   }
+
+  const preprocessedEvent = preprocessResult.internalEvent;
+  debug("preprocessedEvent", preprocessedEvent);
+  const reqProps = {
+    method: preprocessedEvent.method,
+    url: preprocessedEvent.url,
+    //WORKAROUND: We pass this header to the serverless function to mimic a prefetch request which will not trigger revalidation since we handle revalidation differently
+    // There is 3 way we can handle revalidation:
+    // 1. We could just let the revalidation go as normal, but due to race conditions the revalidation will be unreliable
+    // 2. We could alter the lastModified time of our cache to make next believe that the cache is fresh, but this could cause issues with stale data since the cdn will cache the stale data as if it was fresh
+    // 3. OUR CHOICE: We could pass a purpose prefetch header to the serverless function to make next believe that the request is a prefetch request and not trigger revalidation (This could potentially break in the future if next changes the behavior of prefetch requests)
+    headers: { ...headers, purpose: "prefetch" },
+    body: preprocessedEvent.body,
+    remoteAddress: preprocessedEvent.remoteAddress,
+  };
+  const requestId = Math.random().toString(36);
+  const pendingPromiseRunner = new DetachedPromiseRunner();
+  const isISRRevalidation = headers["x-isr"] === "1";
+  const mergeHeadersPriority = globalThis.openNextConfig.dangerous
+    ?.headersAndCookiesPriority
+    ? globalThis.openNextConfig.dangerous.headersAndCookiesPriority(
+        preprocessedEvent,
+      )
+    : "middleware";
+  const internalResult = await globalThis.__als.run(
+    {
+      requestId,
+      pendingPromiseRunner,
+      isISRRevalidation,
+      mergeHeadersPriority,
+    },
+    async () => {
+      const preprocessedResult = preprocessResult as MiddlewareOutputEvent;
+      const req = new IncomingMessage(reqProps);
+      const res = createServerResponse(
+        preprocessedEvent,
+        overwrittenResponseHeaders,
+        responseStreaming,
+      );
+
+      await processRequest(
+        req,
+        res,
+        preprocessedEvent,
+        preprocessedResult.isExternalRewrite,
+      );
+
+      const { statusCode, headers, isBase64Encoded, body } = convertRes(res);
+
+      const internalResult = {
+        type: internalEvent.type,
+        statusCode,
+        headers,
+        body,
+        isBase64Encoded,
+      };
+
+      // reset lastModified. We need to do this to avoid memory leaks
+      delete globalThis.lastModified[requestId];
+
+      await pendingPromiseRunner.await();
+
+      return internalResult;
+    },
+  );
+  return internalResult;
 }
 
 async function processRequest(
@@ -169,18 +168,16 @@ async function processRequest(
     // `serverHandler` is replaced at build time depending on user's
     // nextjs version to patch Nextjs 13.4.x and future breaking changes.
 
-    const { rawPath } = internalEvent;
-
     if (isExternalRewrite) {
       return proxyRequest(internalEvent, res);
-    } else {
-      //#override applyNextjsPrebundledReact
-      setNextjsPrebundledReact(rawPath);
-      //#endOverride
-
-      // Next Server
-      await requestHandler(req, res);
     }
+
+    //#override applyNextjsPrebundledReact
+    setNextjsPrebundledReact(internalEvent.rawPath);
+    //#endOverride
+
+    // Next Server
+    await requestHandler(req, res);
   } catch (e: any) {
     // This might fail when using bundled next, importing won't do the trick either
     if (e.constructor.name === "NoFallbackError") {