Skip to content

Commit

Permalink
Mark node:* imports as external when using the nodejs_compat comp…
Browse files Browse the repository at this point in the history
…atibility flag
  • Loading branch information
GregBrimble committed Feb 27, 2023
1 parent e36e9cf commit 97b4201
Show file tree
Hide file tree
Showing 18 changed files with 255 additions and 22 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.
19 changes: 0 additions & 19 deletions packages/wrangler/index.js

This file was deleted.

15 changes: 15 additions & 0 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
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\\""
`);
});
});
72 changes: 72 additions & 0 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6713,6 +6713,78 @@ export default{
});
});

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
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
12 changes: 12 additions & 0 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 @@ -103,6 +112,7 @@ export async function bundleWorker(
tsconfig?: string;
minify?: boolean;
legacyNodeCompat?: boolean;
nodejsCompat?: boolean;
define: Config["define"];
checkFetch: boolean;
services?: Config["services"];
Expand Down Expand Up @@ -132,6 +142,7 @@ export async function bundleWorker(
tsconfig,
minify,
legacyNodeCompat,
nodejsCompat,
checkFetch,
local,
assets,
Expand Down Expand Up @@ -363,6 +374,7 @@ export async function bundleWorker(
...(legacyNodeCompat
? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()]
: []),
...(nodejsCompat ? [nodejsCompatPlugin] : []),
...(plugins || []),
],
...(jsxFactory && { jsxFactory }),
Expand Down
14 changes: 14 additions & 0 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ export async function startDev(args: StartDevOptions) {
const {
entry,
legacyNodeCompat,
nodejsCompat,
upstreamProtocol,
zoneId,
host,
Expand Down Expand Up @@ -428,6 +429,7 @@ export async function startDev(args: StartDevOptions) {
legacyEnv={isLegacyEnv(configParam)}
minify={args.minify ?? configParam.minify}
legacyNodeCompat={legacyNodeCompat}
nodejsCompat={nodejsCompat}
build={configParam.build || {}}
define={{ ...configParam.define, ...cliDefines }}
initialMode={
Expand Down Expand Up @@ -524,6 +526,7 @@ export async function startApiDev(args: StartDevOptions) {
const {
entry,
legacyNodeCompat,
nodejsCompat,
upstreamProtocol,
zoneId,
host,
Expand Down Expand Up @@ -563,6 +566,7 @@ export async function startApiDev(args: StartDevOptions) {
legacyEnv: isLegacyEnv(configParam),
minify: args.minify ?? configParam.minify,
legacyNodeCompat,
nodejsCompat,
build: configParam.build || {},
define: { ...config.define, ...cliDefines },
initialMode: args.local ? "local" : "remote",
Expand Down Expand Up @@ -749,6 +753,15 @@ async function validateDevServerSettings(
);
}

const compatibilityFlags =
args.compatibilityFlags ?? config.compatibility_flags;
const nodejsCompat = compatibilityFlags?.includes("nodejs_compat");
if (legacyNodeCompat && nodejsCompat) {
throw new Error(
"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."
);
}

if (args.experimentalEnableLocalPersistence) {
logger.warn(
`--experimental-enable-local-persistence is deprecated.\n` +
Expand All @@ -769,6 +782,7 @@ async function validateDevServerSettings(
entry,
upstreamProtocol,
legacyNodeCompat,
nodejsCompat,
getLocalPort,
getInspectorPort,
zoneId,
Expand Down
2 changes: 2 additions & 0 deletions packages/wrangler/src/dev/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export type DevProps = {
usageModel: "bundled" | "unbound" | undefined;
minify: boolean | undefined;
legacyNodeCompat: boolean | undefined;
nodejsCompat: boolean | undefined;
build: Config["build"];
env: string | undefined;
legacyEnv: boolean;
Expand Down Expand Up @@ -279,6 +280,7 @@ function DevSession(props: DevSessionProps) {
tsconfig: props.tsconfig,
minify: props.minify,
legacyNodeCompat: props.legacyNodeCompat,
nodejsCompat: props.nodejsCompat,
betaD1Shims,
define: props.define,
noBundle: props.noBundle,
Expand Down
4 changes: 4 additions & 0 deletions packages/wrangler/src/dev/start-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export async function startDevServer(
tsconfig: props.tsconfig,
minify: props.minify,
legacyNodeCompat: props.legacyNodeCompat,
nodejsCompat: props.nodejsCompat,
define: props.define,
noBundle: props.noBundle,
assets: props.assetsConfig,
Expand Down Expand Up @@ -211,6 +212,7 @@ async function runEsbuild({
tsconfig,
minify,
legacyNodeCompat,
nodejsCompat,
define,
noBundle,
workerDefinitions,
Expand All @@ -234,6 +236,7 @@ async function runEsbuild({
tsconfig: string | undefined;
minify: boolean | undefined;
legacyNodeCompat: boolean | undefined;
nodejsCompat: boolean | undefined;
noBundle: boolean;
workerDefinitions: WorkerRegistry;
firstPartyWorkerDevFacade: boolean | undefined;
Expand Down Expand Up @@ -267,6 +270,7 @@ async function runEsbuild({
tsconfig,
minify,
legacyNodeCompat,
nodejsCompat,
define,
checkFetch: true,
assets: assets && {
Expand Down
4 changes: 4 additions & 0 deletions packages/wrangler/src/dev/use-esbuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function useEsbuild({
tsconfig,
minify,
legacyNodeCompat,
nodejsCompat,
betaD1Shims,
define,
noBundle,
Expand All @@ -55,6 +56,7 @@ export function useEsbuild({
tsconfig: string | undefined;
minify: boolean | undefined;
legacyNodeCompat: boolean | undefined;
nodejsCompat: boolean | undefined;
betaD1Shims?: string[];
noBundle: boolean;
workerDefinitions: WorkerRegistry;
Expand Down Expand Up @@ -122,6 +124,7 @@ export function useEsbuild({
tsconfig,
minify,
legacyNodeCompat,
nodejsCompat,
betaD1Shims,
doBindings: durableObjects.bindings,
define,
Expand Down Expand Up @@ -189,6 +192,7 @@ export function useEsbuild({
noBundle,
minify,
legacyNodeCompat,
nodejsCompat,
define,
assets,
services,
Expand Down
Loading

0 comments on commit 97b4201

Please sign in to comment.