Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for node:* imports with the nodejs_compat compatibility flag #2539

Merged
merged 3 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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": "▲ [WARNING] Enabling 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.
"warn": "▲ [WARNING] Enabling 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.

",
}
Expand Down Expand Up @@ -6705,14 +6705,86 @@ export default{
"err": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "▲ [WARNING] Enabling 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.
"warn": "▲ [WARNING] Enabling 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.

",
}
`);
});
});

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(`
"▲ [WARNING] Enabling 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.
"▲ [WARNING] Enabling 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.


▲ [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(`
"▲ [WARNING] Enabling 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.
"▲ [WARNING] Enabling 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.


▲ [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