From 1e358e249c6b9b1f1c21b858163eee114de2ecc6 Mon Sep 17 00:00:00 2001 From: bcoll Date: Fri, 9 Feb 2024 10:30:58 +0000 Subject: [PATCH 1/3] fix: ensure worker reloads when importing `node:*` modules --- .changeset/cuddly-apples-divide.md | 7 +++++++ packages/wrangler/e2e/dev.test.ts | 7 ++++++- .../esbuild-plugins/nodejs-compat.ts | 18 ++++++++++-------- 3 files changed, 23 insertions(+), 9 deletions(-) create mode 100644 .changeset/cuddly-apples-divide.md diff --git a/.changeset/cuddly-apples-divide.md b/.changeset/cuddly-apples-divide.md new file mode 100644 index 000000000000..675539ca5e64 --- /dev/null +++ b/.changeset/cuddly-apples-divide.md @@ -0,0 +1,7 @@ +--- +"wrangler": patch +--- + +fix: ensure `wrangler dev` can reload without crashing when importing `node:*` modules + +The previous Wrangler release introduced a regression that caused reloads to fail when importing `node:*` modules. This change fixes that, and ensures these modules can always be resolved. diff --git a/packages/wrangler/e2e/dev.test.ts b/packages/wrangler/e2e/dev.test.ts index 9b3b337df66b..99da588adbf7 100644 --- a/packages/wrangler/e2e/dev.test.ts +++ b/packages/wrangler/e2e/dev.test.ts @@ -162,6 +162,7 @@ describe("basic dev tests", () => { name = "${workerName}" main = "src/index.ts" compatibility_date = "2023-01-01" + compatibility_flags = ["nodejs_compat"] [vars] KEY = "value" @@ -195,9 +196,12 @@ describe("basic dev tests", () => { await worker.seed({ "src/index.ts": dedent` + import { Buffer } from "node:buffer"; export default { fetch(request, env) { - return new Response("Updated Worker! " + env.KEY) + const base64Message = Buffer.from("Updated Worker!").toString("base64"); + const message = Buffer.from(base64Message, "base64").toString(); + return new Response(message + " " + env.KEY) } }`, }); @@ -216,6 +220,7 @@ describe("basic dev tests", () => { name = "${workerName}" main = "src/index.ts" compatibility_date = "2023-01-01" + compatibility_flags = ["nodejs_compat"] [vars] KEY = "updated" diff --git a/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts b/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts index 3b6b2094f572..4de00da8534d 100644 --- a/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts +++ b/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts @@ -3,12 +3,6 @@ import chalk from "chalk"; import { logger } from "../../logger"; import type { Plugin } from "esbuild"; -// Infinite loop detection -const seen = new Set(); - -// Prevent multiple warnings per package -const warnedPackaged = new Map(); - /** * An esbuild plugin that will mark any `node:...` imports as external. */ @@ -17,8 +11,16 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = ( ) => ({ name: "nodejs_compat imports plugin", setup(pluginBuild) { - seen.clear(); - warnedPackaged.clear(); + // Infinite loop detection + const seen = new Set(); + + // Prevent multiple warnings per package + const warnedPackaged = new Map(); + + pluginBuild.onStart(() => { + seen.clear(); + warnedPackaged.clear(); + }); pluginBuild.onResolve( { filter: /node:.*/ }, async ({ path, kind, resolveDir, ...opts }) => { From f92a8fbb2257c8ee0b034202a3d4a11d8d4cdf98 Mon Sep 17 00:00:00 2001 From: bcoll Date: Fri, 9 Feb 2024 10:35:40 +0000 Subject: [PATCH 2/3] fix: ensure conflicting compatibility flags reported as user error --- .../src/__tests__/navigator-user-agent.test.ts | 2 +- packages/wrangler/src/navigator-user-agent.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/wrangler/src/__tests__/navigator-user-agent.test.ts b/packages/wrangler/src/__tests__/navigator-user-agent.test.ts index 0f37527f4a27..c305fcc6c3d1 100644 --- a/packages/wrangler/src/__tests__/navigator-user-agent.test.ts +++ b/packages/wrangler/src/__tests__/navigator-user-agent.test.ts @@ -72,7 +72,7 @@ describe("isNavigatorDefined", () => { assert(false, "Unreachable"); } catch (e) { expect(e).toMatchInlineSnapshot( - `[AssertionError: Can't both enable and disable a flag]` + `[Error: Can't both enable and disable a flag]` ); } }); diff --git a/packages/wrangler/src/navigator-user-agent.ts b/packages/wrangler/src/navigator-user-agent.ts index b4f15a0d451a..c0d6a4f41179 100644 --- a/packages/wrangler/src/navigator-user-agent.ts +++ b/packages/wrangler/src/navigator-user-agent.ts @@ -1,16 +1,16 @@ -import assert from "node:assert"; +import { UserError } from "./errors"; export function isNavigatorDefined( compatibility_date: string | undefined, compatibility_flags: string[] = [] ) { - assert( - !( - compatibility_flags.includes("global_navigator") && - compatibility_flags.includes("no_global_navigator") - ), - "Can't both enable and disable a flag" - ); + if ( + compatibility_flags.includes("global_navigator") && + compatibility_flags.includes("no_global_navigator") + ) { + throw new UserError("Can't both enable and disable a flag"); + } + if (compatibility_flags.includes("global_navigator")) { return true; } From a6924996c0f272095daa975b24358b998928d529 Mon Sep 17 00:00:00 2001 From: bcoll Date: Fri, 9 Feb 2024 10:55:39 +0000 Subject: [PATCH 3/3] fix: add type arguments to `seen` and `warnedPackaged` --- .../esbuild-plugins/nodejs-compat.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts b/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts index 4de00da8534d..e87d54da898f 100644 --- a/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts +++ b/packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts @@ -12,10 +12,10 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = ( name: "nodejs_compat imports plugin", setup(pluginBuild) { // Infinite loop detection - const seen = new Set(); + const seen = new Set(); // Prevent multiple warnings per package - const warnedPackaged = new Map(); + const warnedPackaged = new Map(); pluginBuild.onStart(() => { seen.clear(); @@ -41,14 +41,12 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = ( // esbuild couldn't resolve the package // We should warn the user, but not fail the build - if (!warnedPackaged.has(path)) { - warnedPackaged.set(path, [opts.importer]); - } else { - warnedPackaged.set(path, [ - ...warnedPackaged.get(path), - opts.importer, - ]); + let pathWarnedPackaged = warnedPackaged.get(path); + if (pathWarnedPackaged === undefined) { + warnedPackaged.set(path, (pathWarnedPackaged = [])); } + pathWarnedPackaged.push(opts.importer); + return { external: true }; } // This is a normal package—don't treat it specially