make sure all code patch steps are validated

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");
@@ -139,6 +128,7 @@ globalThis.__dangerous_ON_edge_converter_returns_request = true;
     );
   }
 
+  console.log();
   console.log(`\x1b[35mWorker saved in \`${openNextServerBundle}\` 🚀\n\x1b[0m`);
 }
 
@@ -155,25 +145,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 +192,33 @@ 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 functions that take a string (pre-patch code) and return 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 prePatchCode = code;
+  let postPatchCode = code;
+
+  for (const [target, patchFunction] of patches) {
+    console.log(` - patching ${target}`);
+
+    prePatchCode = postPatchCode;
+    postPatchCode = await patchFunction(prePatchCode);
+
+    if (prePatchCode === postPatchCode) {
+      throw new Error(`Failed to patch ${target}`);
+    }
+  }
+
+  return postPatchCode;
+}

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");
 }

packages/cloudflare/src/cli/build/patches/to-investigate/patch-find-dir.ts

@@ -10,8 +10,7 @@ import { Config } from "../../../config";
  * Note: `findDir` uses `fs.existsSync` under the hood, so patching that should be enough to make this work
  */
 export function patchFindDir(code: string, config: Config): string {
-  console.log("# patchFindDir");
-  const patchedCode = code.replace(
+  return code.replace(
     /function findDir\((?<dir>dir\d*), (?<name>name\d*)\) {/,
     `function findDir($dir, $name) {
 			if ($dir.endsWith(".next/server")) {
@@ -25,10 +24,4 @@ export function patchFindDir(code: string, config: Config): string {
 		throw new Error("Unknown findDir call: " + $dir + " " + $name);
 		`
   );
-
-  if (patchedCode === code) {
-    throw new Error("Patch `patchFindDir` not applied");
-  }
-
-  return patchedCode;
 }

packages/cloudflare/src/cli/build/patches/to-investigate/patch-load-instrumentation-module.ts

@@ -34,6 +34,5 @@ export function patchLoadInstrumentationModule(code: string) {
   loadInstrumentationModuleDeclarations.forEach((loadInstrumentationModuleDeclaration) => {
     loadInstrumentationModuleDeclaration.setBodyText("");
   });
-
   return file.print();
 }

packages/cloudflare/src/cli/build/patches/to-investigate/patch-read-file.ts

@@ -6,28 +6,29 @@ import { globSync } from "glob";
 import { Config } from "../../../config";
 import { normalizePath } from "../../utils";
 
-export function patchReadFile(code: string, config: Config): string {
-  console.log("# patchReadFile");
+export function patchBuildId(code: string, config: Config): string {
   // The next-server code gets the buildId from the filesystem, resulting in a `[unenv] fs.readFileSync is not implemented yet!` error
   // so we add an early return to the `getBuildId` function so that the `readyFileSync` is never encountered
   // (source: https://github.com/vercel/next.js/blob/15aeb92efb34c09a36/packages/next/src/server/next-server.ts#L438-L451)
   // Note: we could/should probably just patch readFileSync here or something!
-  code = code.replace(
+  return code.replace(
     "getBuildId() {",
     `getBuildId() {
       return ${JSON.stringify(readFileSync(join(config.paths.output.standaloneAppDotNext, "BUILD_ID"), "utf-8"))};
     `
   );
+}
 
-  // Same as above, the next-server code loads the manifests with `readFileSync` and we want to avoid that
+export function patchLoadManifest(code: string, config: Config): string {
+  // Same as patchBuildId, the next-server code loads the manifests with `readFileSync` and we want to avoid that
   // (source: https://github.com/vercel/next.js/blob/15aeb92e/packages/next/src/server/load-manifest.ts#L34-L56)
   // Note: we could/should probably just patch readFileSync here or something!
   const manifestJsons = globSync(
     normalizePath(join(config.paths.output.standaloneAppDotNext, "**", "*-manifest.json"))
   ).map((file) =>
     normalizePath(file).replace(normalizePath(config.paths.output.standaloneApp) + posix.sep, "")
   );
-  code = code.replace(
+  return code.replace(
     /function loadManifest\((.+?), .+?\) {/,
     `$&
     ${manifestJsons
@@ -42,6 +43,4 @@ export function patchReadFile(code: string, config: Config): string {
     throw new Error("Unknown loadManifest: " + $1);
     `
   );
-
-  return code;
 }