nodejs compat functions build (#3133)

cloudflare · May 5, 2023 · d078800 · d078800
1 parent 5079f47
commit d078800
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 5 deletions.
diff --git a/.changeset/metal-buckets-fly.md b/.changeset/metal-buckets-fly.md
@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix pages building not taking into account the nodejs_compat flag (and improve the related error message)
diff --git a/packages/wrangler/src/__tests__/pages/functions-build.test.ts b/packages/wrangler/src/__tests__/pages/functions-build.test.ts
@@ -448,6 +448,12 @@ export default {
 		"Build failed with 1 error:
 		hello.js:2:36: ERROR: Could not resolve \\"node:async_hooks\\""
 	`);
+		expect(std.err).toContain(
+			'The package "node:async_hooks" wasn\'t found on the file system but is built into node.'
+		);
+		expect(std.err).toContain(
+			'Add the "nodejs_compat" compatibility flag to your Pages project to enable Node.js compatibility.'
+		);
 	});
 
 	it("should compile a _worker.js/ directory", async () => {

diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts
@@ -7224,7 +7224,7 @@ export default{
 			expect(
 				esbuild.formatMessagesSync(err?.errors ?? [], { kind: "error" }).join()
 			).toMatch(
-				/The package "path" wasn't found on the file system but is built into node\.\s+Add "node_compat = true" to your wrangler\.toml file to enable Node compatibility\./
+				/The package "path" wasn't found on the file system but is built into node\.\s+Add "node_compat = true" to your wrangler\.toml file to enable Node.js compatibility\./
 			);
 		});
 

diff --git a/packages/wrangler/src/bundle.ts b/packages/wrangler/src/bundle.ts
@@ -77,15 +77,25 @@ export function isBuildFailure(err: unknown): err is esbuild.BuildFailure {
  * Rewrites esbuild BuildFailures for failing to resolve Node built-in modules
  * to suggest enabling Node compat as opposed to `platform: "node"`.
  */
-export function rewriteNodeCompatBuildFailure(err: esbuild.BuildFailure) {
+export function rewriteNodeCompatBuildFailure(
+	err: esbuild.BuildFailure,
+	forPages = false
+) {
 	for (const error of err.errors) {
 		const match = nodeBuiltinResolveErrorText.exec(error.text);
 		if (match !== null) {
+			const issue = `The package "${match[1]}" wasn't found on the file system but is built into node.`;
+
+			const instructionForUser = `${
+				forPages
+					? 'Add the "nodejs_compat" compatibility flag to your Pages project'
+					: 'Add "node_compat = true" to your wrangler.toml file'
+			} to enable Node.js compatibility.`;
+
 			error.notes = [
 				{
 					location: null,
-					text: `The package "${match[1]}" wasn't found on the file system but is built into node.
-Add "node_compat = true" to your wrangler.toml file to enable Node compatibility.`,
+					text: `${issue}\n${instructionForUser}`,
 				},
 			];
 		}
@@ -148,6 +158,7 @@ export async function bundleWorker(
 		// TODO: Rip these out https://github.com/cloudflare/workers-sdk/issues/2153
 		disableModuleCollection?: boolean;
 		isOutfile?: boolean;
+		forPages?: boolean;
 	}
 ): Promise<BundleResult> {
 	const {
@@ -179,6 +190,7 @@ export async function bundleWorker(
 		plugins,
 		disableModuleCollection,
 		isOutfile,
+		forPages,
 	} = options;
 
 	// We create a temporary directory for any oneoff files we
@@ -412,7 +424,7 @@ export async function bundleWorker(
 		result = await esbuild.build(buildOptions);
 	} catch (e) {
 		if (!legacyNodeCompat && isBuildFailure(e))
-			rewriteNodeCompatBuildFailure(e);
+			rewriteNodeCompatBuildFailure(e, forPages);
 		throw e;
 	}
 

diff --git a/packages/wrangler/src/pages/build.ts b/packages/wrangler/src/pages/build.ts
@@ -233,6 +233,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 					sourcemap,
 					watch,
 					betaD1Shims: d1Databases,
+					nodejsCompat,
 				});
 			}
 		} else {

diff --git a/packages/wrangler/src/pages/functions/buildPlugin.ts b/packages/wrangler/src/pages/functions/buildPlugin.ts
@@ -108,6 +108,7 @@ export function buildPlugin({
 			targetConsumer: local ? "dev" : "publish",
 			local,
 			experimentalLocal: false,
+			forPages: true,
 		}
 	);
 }
diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts
@@ -153,6 +153,7 @@ export function buildWorker({
 			targetConsumer: local ? "dev" : "publish",
 			local,
 			experimentalLocal: false,
+			forPages: true,
 		}
 	);
 }
@@ -227,6 +228,7 @@ export function buildRawWorker({
 			targetConsumer: local ? "dev" : "publish",
 			local,
 			experimentalLocal: false,
+			forPages: true,
 		}
 	);
 }