make sure all code patch steps are validated (#186)

make sure all code patch steps are validated and improve the code patching validation by removing the validation for each single patch function and introducing a new `patchCodeWithValidations` that applies all the patches
with validations all in a single place

Author: dario-piotrowicz

packages/cloudflare/src/cli/build/bundle-server.ts

@@ -7,18 +7,7 @@ import type { BuildOptions } from "@opennextjs/aws/build/helper.js";
 import { build, Plugin } from "esbuild";
 
 import { Config } from "../config";
-import { copyPackageCliFiles } from "./patches/investigated/copy-package-cli-files";
-import { patchCache } from "./patches/investigated/patch-cache";
-import { patchRequire } from "./patches/investigated/patch-require";
-import { updateWebpackChunksFile } from "./patches/investigated/update-webpack-chunks-file";
-import { inlineEvalManifest } from "./patches/to-investigate/inline-eval-manifest";
-import { inlineMiddlewareManifestRequire } from "./patches/to-investigate/inline-middleware-manifest-require";
-import { inlineNextRequire } from "./patches/to-investigate/inline-next-require";
-import { patchExceptionBubbling } from "./patches/to-investigate/patch-exception-bubbling";
-import { patchFindDir } from "./patches/to-investigate/patch-find-dir";
-import { patchLoadInstrumentationModule } from "./patches/to-investigate/patch-load-instrumentation-module";
-import { patchReadFile } from "./patches/to-investigate/patch-read-file";
-import { patchWranglerDeps } from "./patches/to-investigate/wrangler-deps";
+import * as patches from "./patches";
 import { copyPrerenderedRoutes } from "./utils";
 
 /** The dist directory of the Cloudflare adapter package */
@@ -31,7 +20,7 @@ export async function bundleServer(config: Config, openNextOptions: BuildOptions
   // Copy over prerendered assets (e.g. SSG routes)
   copyPrerenderedRoutes(config);
 
-  copyPackageCliFiles(packageDistDir, config, openNextOptions);
+  patches.copyPackageCliFiles(packageDistDir, config, openNextOptions);
 
   const nextConfigStr =
     fs
@@ -40,8 +29,8 @@ export async function bundleServer(config: Config, openNextOptions: BuildOptions
 
   console.log(`\x1b[35m⚙️ Bundling the OpenNext server...\n\x1b[0m`);
 
-  patchWranglerDeps(config);
-  updateWebpackChunksFile(config);
+  patches.patchWranglerDeps(config);
+  patches.updateWebpackChunksFile(config);
 
   const { appBuildOutputPath, appPath, outputDir, monorepoRoot } = openNextOptions;
   const outputPath = path.join(outputDir, "server-functions", "default");
@@ -155,25 +144,35 @@ async function updateWorkerBundledCode(
   config: Config,
   openNextOptions: BuildOptions
 ): Promise<void> {
-  const originalCode = await readFile(workerOutputFile, "utf8");
-
-  let patchedCode = originalCode;
-
-  patchedCode = patchRequire(patchedCode);
-  patchedCode = patchReadFile(patchedCode, config);
-  patchedCode = inlineNextRequire(patchedCode, config);
-  patchedCode = patchFindDir(patchedCode, config);
-  patchedCode = inlineEvalManifest(patchedCode, config);
-  patchedCode = await patchCache(patchedCode, openNextOptions);
-  patchedCode = inlineMiddlewareManifestRequire(patchedCode, config);
-  patchedCode = patchExceptionBubbling(patchedCode);
-  patchedCode = patchLoadInstrumentationModule(patchedCode);
-
-  patchedCode = patchedCode
-    // workers do not support dynamic require nor require.resolve
-    // TODO: implement for cf (possibly in @opennextjs/aws)
-    .replace("patchAsyncStorage();", "//patchAsyncStorage();")
-    .replace('require.resolve("./cache.cjs")', '"unused"');
+  const code = await readFile(workerOutputFile, "utf8");
+
+  const patchedCode = await patchCodeWithValidations(code, [
+    ["require", patches.patchRequire],
+    ["`buildId` function", (code) => patches.patchBuildId(code, config)],
+    ["`loadManifest` function", (code) => patches.patchLoadManifest(code, config)],
+    ["next's require", (code) => patches.inlineNextRequire(code, config)],
+    ["`findDir` function", (code) => patches.patchFindDir(code, config)],
+    ["`evalManifest` function", (code) => patches.inlineEvalManifest(code, config)],
+    ["cacheHandler", (code) => patches.patchCache(code, openNextOptions)],
+    [
+      "'require(this.middlewareManifestPath)'",
+      (code) => patches.inlineMiddlewareManifestRequire(code, config),
+    ],
+    ["exception bubbling", patches.patchExceptionBubbling],
+    ["`loadInstrumentationModule` function", patches.patchLoadInstrumentationModule],
+    [
+      "`patchAsyncStorage` call",
+      (code) =>
+        code
+          // TODO: implement for cf (possibly in @opennextjs/aws)
+          .replace("patchAsyncStorage();", "//patchAsyncStorage();"),
+    ],
+    [
+      "`require.resolve` call",
+      // workers do not support dynamic require nor require.resolve
+      (code) => code.replace('require.resolve("./cache.cjs")', '"unused"'),
+    ],
+  ]);
 
   await writeFile(workerOutputFile, patchedCode);
 }
@@ -192,3 +191,34 @@ function createFixRequiresESBuildPlugin(config: Config): Plugin {
     },
   };
 }
+
+/**
+ * Applies multiple code patches in order to a given piece of code, at each step it validates that the code
+ * has actually been patched/changed, if not an error is thrown
+ *
+ * @param code the code to apply the patches to
+ * @param patches array of tuples, containing a string indicating the target of the patching (for logging) and
+ *                a patching function that takes a string (pre-patch code) and returns a string (post-patch code)
+ * @returns the patched code
+ */
+async function patchCodeWithValidations(
+  code: string,
+  patches: [string, (code: string) => string | Promise<string>][]
+): Promise<string> {
+  console.log(`Applying code patches:`);
+  let patchedCode = code;
+
+  for (const [target, patchFunction] of patches) {
+    console.log(` - patching ${target}`);
+
+    const prePatchCode = patchedCode;
+    patchedCode = await patchFunction(patchedCode);
+
+    if (prePatchCode === patchedCode) {
+      throw new Error(`Failed to patch ${target}`);
+    }
+  }
+
+  console.log(`All ${patches.length} patches applied\n`);
+  return patchedCode;
+}

packages/cloudflare/src/cli/build/patches/index.ts

@@ -0,0 +1,2 @@
+export * from "./investigated";
+export * from "./to-investigate";

packages/cloudflare/src/cli/build/patches/investigated/index.ts

@@ -0,0 +1,4 @@
+export * from "./copy-package-cli-files";
+export * from "./patch-cache";
+export * from "./patch-require";
+export * from "./update-webpack-chunks-file";

packages/cloudflare/src/cli/build/patches/investigated/patch-cache.ts

@@ -14,25 +14,17 @@ import type { BuildOptions } from "@opennextjs/aws/build/helper.js";
  * instantiated with a dynamic require that uses a string literal for the path.
  */
 export async function patchCache(code: string, openNextOptions: BuildOptions): Promise<string> {
-  console.log("# patchCache");
-
   const { appBuildOutputPath, outputDir, monorepoRoot } = openNextOptions;
 
   // TODO: switch to cache.mjs
   const outputPath = path.join(outputDir, "server-functions", "default");
   const packagePath = path.relative(monorepoRoot, appBuildOutputPath);
   const cacheFile = path.join(outputPath, packagePath, "cache.cjs");
 
-  const patchedCode = code.replace(
+  return code.replace(
     "const { cacheHandler } = this.nextConfig;",
     `const cacheHandler = null;
 CacheHandler = require('${cacheFile}').default;
 `
   );
-
-  if (patchedCode === code) {
-    throw new Error("Patch `patchCache` not applied");
-  }
-
-  return patchedCode;
 }

packages/cloudflare/src/cli/build/patches/investigated/patch-require.ts

@@ -5,6 +5,5 @@
  * James on Aug 29: `module.createRequire()` is planned.
  */
 export function patchRequire(code: string): string {
-  console.log("# patchRequire");
   return code.replace(/__require\d?\(/g, "require(").replace(/__require\d?\./g, "require.");
 }

packages/cloudflare/src/cli/build/patches/to-investigate/index.ts

@@ -0,0 +1,8 @@
+export * from "./inline-eval-manifest";
+export * from "./inline-middleware-manifest-require";
+export * from "./inline-next-require";
+export * from "./patch-exception-bubbling";
+export * from "./patch-find-dir";
+export * from "./patch-load-instrumentation-module";
+export * from "./patch-read-file";
+export * from "./wrangler-deps";

packages/cloudflare/src/cli/build/patches/to-investigate/inline-eval-manifest.ts

@@ -13,7 +13,6 @@ import { normalizePath } from "../../utils";
  * there is a vm `runInNewContext` call which we also don't support (source: https://github.com/vercel/next.js/blob/b1e32c5d1f/packages/next/src/server/load-manifest.ts#L88)
  */
 export function inlineEvalManifest(code: string, config: Config): string {
-  console.log("# inlineEvalManifest");
   const manifestJss = globSync(
     normalizePath(join(config.paths.output.standaloneAppDotNext, "**", "*_client-reference-manifest.js"))
   ).map((file) =>

packages/cloudflare/src/cli/build/patches/to-investigate/inline-middleware-manifest-require.ts

@@ -8,22 +8,11 @@ import { Config } from "../../../config";
  * as they result in runtime failures.
  */
 export function inlineMiddlewareManifestRequire(code: string, config: Config) {
-  console.log("# inlineMiddlewareManifestRequire");
-
   const middlewareManifestPath = join(config.paths.output.standaloneAppServer, "middleware-manifest.json");
 
   const middlewareManifest = existsSync(middlewareManifestPath)
     ? JSON.parse(readFileSync(middlewareManifestPath, "utf-8"))
     : {};
 
-  const patchedCode = code.replace(
-    "require(this.middlewareManifestPath)",
-    JSON.stringify(middlewareManifest)
-  );
-
-  if (patchedCode === code) {
-    throw new Error("Patch `inlineMiddlewareManifestRequire` not applied");
-  }
-
-  return patchedCode;
+  return code.replace("require(this.middlewareManifestPath)", JSON.stringify(middlewareManifest));
 }

packages/cloudflare/src/cli/build/patches/to-investigate/inline-next-require.ts

@@ -8,7 +8,6 @@ import { Config } from "../../../config";
  * and inline their content during build time
  */
 export function inlineNextRequire(code: string, config: Config) {
-  console.log("# inlineNextRequire");
   const pagesManifestFile = join(config.paths.output.standaloneAppServer, "pages-manifest.json");
   const appPathsManifestFile = join(config.paths.output.standaloneAppServer, "app-paths-manifest.json");
 

packages/cloudflare/src/cli/build/patches/to-investigate/patch-exception-bubbling.ts

@@ -5,13 +5,5 @@
  * promises.
  */
 export function patchExceptionBubbling(code: string) {
-  console.log("# patchExceptionBubbling");
-
-  const patchedCode = code.replace('_nextBubbleNoFallback = "1"', "_nextBubbleNoFallback = undefined");
-
-  if (patchedCode === code) {
-    throw new Error("Patch `patchExceptionBubbling` not applied");
-  }
-
-  return patchedCode;
+  return code.replace('_nextBubbleNoFallback = "1"', "_nextBubbleNoFallback = undefined");
 }