Skip to content

Commit

Permalink
Add support for node:* imports with the nodejs_compat compatibili…
Browse files Browse the repository at this point in the history
…ty flag (#2539)

* Fix capitalization of Node.js

* Internally rename `nodeCompat` to `legacyNodeCompat`

* Mark `node:*` imports as external when using the `nodejs_compat` compatibility flag
  • Loading branch information
GregBrimble authored Feb 27, 2023
1 parent f7d49eb commit 3725086
Show file tree
Hide file tree
Showing 23 changed files with 324 additions and 71 deletions.
7 changes: 7 additions & 0 deletions .changeset/rare-houses-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

feat: Add support for the `nodejs_compat` Compatibility Flag when bundling a Worker with Wrangler

This new Compatibility Flag is incompatible with the legacy `--node-compat` CLI argument and `node_compat` configuration option. If you want to use the new runtime Node.js compatibility features, please remove the `--node-compat` argument from your CLI command or your config file.
2 changes: 1 addition & 1 deletion .github/workflows/test-old-node-error.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Test old node.js version
name: Test old Node.js version

on: pull_request

Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/bin/wrangler.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function runWrangler() {
// Note Volta and nvm are also recommended in the official docs:
// https://developers.cloudflare.com/workers/get-started/guide#2-install-the-workers-cli
console.error(
`Wrangler requires at least node.js v${MIN_NODE_VERSION}. You are using v${process.versions.node}. Please update your version of node.js.
`Wrangler requires at least Node.js v${MIN_NODE_VERSION}. You are using v${process.versions.node}. Please update your version of Node.js.
Consider using a Node.js version manager such as https://volta.sh/ or https://github.com/nvm-sh/nvm.`
);
Expand Down
17 changes: 16 additions & 1 deletion packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ describe("wrangler dev", () => {
--experimental-local Run on my machine using the Cloudflare Workers runtime [boolean] [default: false]
--experimental-local-remote-kv Read/write KV data from/to real namespaces on the Cloudflare network [boolean] [default: false]
--minify Minify the script [boolean]
--node-compat Enable node.js compatibility [boolean]
--node-compat Enable Node.js compatibility [boolean]
--persist Enable persistence for local mode, using default path: .wrangler/state [boolean]
--persist-to Specify directory to use for local persistence (implies --persist) [string]
--live-reload Auto reload HTML pages when change is detected in local mode [boolean]
Expand Down Expand Up @@ -1566,6 +1566,21 @@ describe("wrangler dev", () => {
`);
});
});

describe("`nodejs_compat` compatibility flag", () => {
it("should conflict with the --node-compat option", async () => {
writeWranglerToml();
fs.writeFileSync("index.js", `export default {};`);

await expect(
runWrangler(
"dev index.js --compatibility-flag=nodejs_compat --node-compat"
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The \`nodejs_compat\` compatibility flag cannot be used in conjunction with the legacy \`--node-compat\` flag. If you want to use the Workers runtime Node.js compatibility features, please remove the \`--node-compat\` argument from your CLI command or \`node_compat = true\` from your config file."`
);
});
});
});

function mockGetZones(domain: string, zones: { id: string }[] = []) {
Expand Down
51 changes: 51 additions & 0 deletions packages/wrangler/src/__tests__/pages/functions-build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,4 +399,55 @@ export default {

expect(std.err).toMatchInlineSnapshot(`""`);
});

it("should leave Node.js imports when the `nodejs_compat` compatibility flag is set", async () => {
mkdirSync("functions");
writeFileSync(
"functions/hello.js",
`
import { AsyncLocalStorage } from 'node:async_hooks';
export async function onRequest() {
console.log(AsyncLocalStorage);
return new Response("Hello from Pages Functions");
}
`
);

await runWrangler(
`pages functions build --outfile=public/_worker.js --compatibility-flag=nodejs_compat`
);

expect(existsSync("public/_worker.js")).toBe(true);
expect(std.out).toMatchInlineSnapshot(`
"🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/workers-sdk/issues/new/choose
✨ Compiled Worker successfully"
`);

expect(readFileSync("public/_worker.js", "utf-8")).toContain(
`import { AsyncLocalStorage } from "node:async_hooks";`
);
});

it("should error at Node.js imports when the `nodejs_compat` compatibility flag is not set", async () => {
mkdirSync("functions");
writeFileSync(
"functions/hello.js",
`
import { AsyncLocalStorage } from 'node:async_hooks';
export async function onRequest() {
console.log(AsyncLocalStorage);
return new Response("Hello from Pages Functions");
}
`
);

await expect(
runWrangler(`pages functions build --outfile=public/_worker.js`)
).rejects.toThrowErrorMatchingInlineSnapshot(`
"Build failed with 1 error:
hello.js:2:36: ERROR: Could not resolve \\"node:async_hooks\\""
`);
});
});
80 changes: 76 additions & 4 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6658,7 +6658,7 @@ export default{
"err": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
"warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
",
}
Expand Down Expand Up @@ -6705,14 +6705,86 @@ export default{
"err": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
"warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
",
}
`);
});
});

describe("`nodejs_compat` compatibility flag", () => {
it('when absent, should error on any "external" `node:*` imports', async () => {
writeWranglerToml();
fs.writeFileSync(
"index.js",
`
import AsyncHooks from 'node:async_hooks';
console.log(AsyncHooks);
export default {}
`
);
let err: esbuild.BuildFailure | undefined;
try {
await runWrangler("publish index.js --dry-run"); // expecting this to throw, as node compatibility isn't enabled
} catch (e) {
err = e as esbuild.BuildFailure;
}
expect(
esbuild.formatMessagesSync(err?.errors ?? [], { kind: "error" }).join()
).toMatch(/Could not resolve "node:async_hooks"/);
});

it('when present, should support any "external" `node:*` imports', async () => {
writeWranglerToml();
fs.writeFileSync(
"index.js",
`
import AsyncHooks from 'node:async_hooks';
console.log(AsyncHooks);
export default {}
`
);

await runWrangler(
"publish index.js --dry-run --outdir=dist --compatibility-flag=nodejs_compat"
);

expect(std).toMatchInlineSnapshot(`
Object {
"debug": "",
"err": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "",
}
`);
expect(fs.readFileSync("dist/index.js", { encoding: "utf-8" })).toContain(
`import AsyncHooks from "node:async_hooks";`
);
});

it("should conflict with the --node-compat option", async () => {
writeWranglerToml();
fs.writeFileSync(
"index.js",
`
import AsyncHooks from 'node:async_hooks';
console.log(AsyncHooks);
export default {}
`
);

await expect(
runWrangler(
"publish index.js --dry-run --outdir=dist --compatibility-flag=nodejs_compat --node-compat"
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`"The \`nodejs_compat\` compatibility flag cannot be used in conjunction with the legacy \`--node-compat\` flag. If you want to use the Workers runtime Node.js compatibility features, please remove the \`--node-compat\` argument from your CLI command or \`node_compat = true\` from your config file."`
);
});
});

describe("bundle reporter", () => {
it("should print the bundle size", async () => {
fs.writeFileSync(
Expand Down Expand Up @@ -7125,7 +7197,7 @@ export default{
"publish index.js --no-bundle --node-compat --dry-run --outdir dist"
);
expect(std.warn).toMatchInlineSnapshot(`
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
▲ [WARNING] \`--node-compat\` and \`--no-bundle\` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process.
Expand All @@ -7145,7 +7217,7 @@ export default{
fs.writeFileSync("index.js", scriptContent);
await runWrangler("publish index.js --dry-run --outdir dist");
expect(std.warn).toMatchInlineSnapshot(`
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
▲ [WARNING] \`--node-compat\` and \`--no-bundle\` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process.
Expand Down
6 changes: 3 additions & 3 deletions packages/wrangler/src/__tests__/test-old-node-version.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// this test has to be run with a version of node.js older than 16.13 to pass
// this test has to be run with a version of Node.js older than 16.13 to pass

const { spawn } = require("child_process");
const path = require("path");
Expand All @@ -11,7 +11,7 @@ const wranglerProcess = spawn(
{ stdio: "pipe" }
);

const messageToMatch = "Wrangler requires at least node.js v16.13.0";
const messageToMatch = "Wrangler requires at least Node.js v16.13.0";

wranglerProcess.once("exit", (code) => {
try {
Expand All @@ -25,7 +25,7 @@ wranglerProcess.once("exit", (code) => {
} catch (err) {
console.error("Error:", err);
throw new Error(
"This test has to be run with a version of node.js under 16.13 to pass"
"This test has to be run with a version of Node.js under 16.13 to pass"
);
}
});
6 changes: 3 additions & 3 deletions packages/wrangler/src/api/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface UnstableDevOptions {
site?: string; // Root folder of static assets for Workers Sites
siteInclude?: string[]; // Array of .gitignore-style patterns that match file or directory names from the sites directory. Only matched items will be uploaded.
siteExclude?: string[]; // Array of .gitignore-style patterns that match file or directory names from the sites directory. Matched items will not be uploaded.
nodeCompat?: boolean; // Enable node.js compatibility
nodeCompat?: boolean; // Enable Node.js compatibility
compatibilityDate?: string; // Date to use for compatibility checks
compatibilityFlags?: string[]; // Flags to use for compatibility checks
persist?: boolean; // Enable persistence for local mode, using default path: .wrangler/state
Expand Down Expand Up @@ -163,7 +163,7 @@ export async function unstable_dev(
site: options?.site, // Root folder of static assets for Workers Sites
siteInclude: options?.siteInclude, // Array of .gitignore-style patterns that match file or directory names from the sites directory. Only matched items will be uploaded.
siteExclude: options?.siteExclude, // Array of .gitignore-style patterns that match file or directory names from the sites directory. Matched items will not be uploaded.
nodeCompat: options?.nodeCompat, // Enable node.js compatibility
nodeCompat: options?.nodeCompat, // Enable Node.js compatibility
persist: options?.persist, // Enable persistence for local mode, using default path: .wrangler/state
persistTo: options?.persistTo, // Specify directory to use for local persistence (implies --persist)
experimentalJsonConfig: undefined,
Expand Down Expand Up @@ -252,7 +252,7 @@ export async function unstable_dev(
site: options?.site, // Root folder of static assets for Workers Sites
siteInclude: options?.siteInclude, // Array of .gitignore-style patterns that match file or directory names from the sites directory. Only matched items will be uploaded.
siteExclude: options?.siteExclude, // Array of .gitignore-style patterns that match file or directory names from the sites directory. Matched items will not be uploaded.
nodeCompat: options?.nodeCompat, // Enable node.js compatibility
nodeCompat: options?.nodeCompat, // Enable Node.js compatibility
persist: options?.persist, // Enable persistence for local mode, using default path: .wrangler/state
persistTo: options?.persistTo, // Specify directory to use for local persistence (implies --persist)
experimentalJsonConfig: undefined,
Expand Down
7 changes: 7 additions & 0 deletions packages/wrangler/src/api/pages/publish.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ export async function publish({
isProduction = project.production_branch === branch;
}

const deploymentConfig =
project.deployment_configs[isProduction ? "production" : "preview"];
const nodejsCompat =
deploymentConfig.compatibility_flags?.includes("nodejs_compat");

/**
* Evaluate if this is an Advanced Mode or Pages Functions project. If Advanced Mode, we'll
* go ahead and upload `_worker.js` as is, but if Pages Functions, we need to attempt to build
Expand Down Expand Up @@ -176,6 +181,7 @@ export async function publish({
routesOutputPath,
local: false,
d1Databases,
nodejsCompat,
experimentalWorkerBundle,
});

Expand Down Expand Up @@ -261,6 +267,7 @@ export async function publish({
watch: false,
onEnd: () => {},
betaD1Shims: d1Databases,
nodejsCompat,
experimentalWorkerBundle,
});
workerFileContents = readFileSync(outfile, "utf8");
Expand Down
23 changes: 18 additions & 5 deletions packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ Add "node_compat = true" to your wrangler.toml file to enable Node compatibility
}
}

const nodejsCompatPlugin: esbuild.Plugin = {
name: "nodejs_compat Plugin",
setup(pluginBuild) {
pluginBuild.onResolve({ filter: /node:.*/ }, () => {
return { external: true };
});
},
};

/**
* Generate a bundle for the worker identified by the arguments passed in.
*/
Expand All @@ -102,7 +111,8 @@ export async function bundleWorker(
watch?: esbuild.WatchMode | boolean;
tsconfig?: string;
minify?: boolean;
nodeCompat?: boolean;
legacyNodeCompat?: boolean;
nodejsCompat?: boolean;
define: Config["define"];
checkFetch: boolean;
services?: Config["services"];
Expand Down Expand Up @@ -131,7 +141,8 @@ export async function bundleWorker(
watch,
tsconfig,
minify,
nodeCompat,
legacyNodeCompat,
nodejsCompat,
checkFetch,
local,
assets,
Expand Down Expand Up @@ -347,7 +358,7 @@ export async function bundleWorker(
// use process.env["NODE_ENV" + ""] so that esbuild doesn't replace it
// when we do a build of wrangler. (re: https://github.com/cloudflare/workers-sdk/issues/1477)
"process.env.NODE_ENV": `"${process.env["NODE_ENV" + ""]}"`,
...(nodeCompat ? { global: "globalThis" } : {}),
...(legacyNodeCompat ? { global: "globalThis" } : {}),
...(checkFetch ? { fetch: "checkedFetch" } : {}),
...options.define,
},
Expand All @@ -360,9 +371,10 @@ export async function bundleWorker(
},
plugins: [
moduleCollector.plugin,
...(nodeCompat
...(legacyNodeCompat
? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()]
: []),
...(nodejsCompat ? [nodejsCompatPlugin] : []),
...(plugins || []),
],
...(jsxFactory && { jsxFactory }),
Expand All @@ -378,7 +390,8 @@ export async function bundleWorker(
try {
result = await esbuild.build(buildOptions);
} catch (e) {
if (!nodeCompat && isBuildFailure(e)) rewriteNodeCompatBuildFailure(e);
if (!legacyNodeCompat && isBuildFailure(e))
rewriteNodeCompatBuildFailure(e);
throw e;
}

Expand Down
Loading

0 comments on commit 3725086

Please sign in to comment.