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 1b3943e
Show file tree
Hide file tree
Showing 16 changed files with 204 additions and 3 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.
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
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
9 changes: 9 additions & 0 deletions packages/wrangler/src/dev/validate-dev-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,13 @@ export function validateDevProps(props: DevProps) {
"You cannot configure [data_blobs] with an ES module worker. Instead, import the file directly in your code, and optionally configure `[rules]` in your wrangler.toml"
);
}

if (
props.compatibilityFlags?.includes("nodejs_compat") &&
props.legacyNodeCompat
) {
throw new Error(
"You cannot use the `nodejs_compat` compatibility flag in conjunction with the legacy `--node-compat` flag. 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."
);
}
}
20 changes: 20 additions & 0 deletions packages/wrangler/src/pages/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ export function Options(yargs: CommonYargsArgv) {
type: "boolean",
hidden: true,
},
"compatibility-date": {
describe: "Date to use for compatibility checks",
type: "string",
requiresArg: true,
},
"compatibility-flags": {
describe: "Flags to use for compatibility checks",
alias: "compatibility-flag",
type: "string",
requiresArg: true,
array: true,
},
bindings: {
type: "string",
describe:
Expand Down Expand Up @@ -110,6 +122,7 @@ export const Handler = async ({
plugin,
buildOutputDirectory,
nodeCompat: legacyNodeCompat,
compatibilityFlags,
bindings,
experimentalWorkerBundle,
}: PagesBuildArgs) => {
Expand All @@ -123,6 +136,12 @@ export const Handler = async ({
"Enabling Node.js compatibility mode for builtins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details."
);
}
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."
);
}

let d1Databases: string[] | undefined = undefined;
if (bindings) {
Expand Down Expand Up @@ -200,6 +219,7 @@ We first looked inside the build output directory (${basename(
plugin,
buildOutputDirectory,
legacyNodeCompat,
nodejsCompat,
routesOutputPath,
local: false,
d1Databases,
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/src/pages/buildFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export async function buildFunctions({
buildOutputDirectory,
routesOutputPath,
legacyNodeCompat,
nodejsCompat,
local,
d1Databases,
experimentalWorkerBundle = false,
Expand All @@ -54,6 +55,7 @@ export async function buildFunctions({
d1Databases?: string[];
experimentalWorkerBundle?: boolean;
legacyNodeCompat?: boolean;
nodejsCompat?: boolean;
}) {
RUNNING_BUILDERS.forEach(
(runningBuilder) => runningBuilder.stop && runningBuilder.stop()
Expand Down Expand Up @@ -121,6 +123,7 @@ export async function buildFunctions({
onEnd,
buildOutputDirectory,
legacyNodeCompat,
nodejsCompat,
experimentalWorkerBundle,
});
}
Expand Down
Loading

0 comments on commit 1b3943e

Please sign in to comment.