Skip to content

Commit

Permalink
feat: recommend node compat on builtin use (#1027)
Browse files Browse the repository at this point in the history
* feat: trying to use node builtins should recommend you enable node_compat

* chore: add example node app to test node_compat against

* Create tricky-actors-pretend.md

* address PR feedback

* chore: add test

* chore: remove unused example app

* Update .changeset/tricky-actors-pretend.md

Co-authored-by: Sunil Pai <[email protected]>

Co-authored-by: Sunil Pai <[email protected]>
  • Loading branch information
rozenmd and threepointone authored May 17, 2022
1 parent 53033f3 commit 3545e41
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/tricky-actors-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

feat: trying to use node builtins should recommend you enable node_compat in wrangler.toml
21 changes: 21 additions & 0 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4658,6 +4658,27 @@ addEventListener('fetch', event => {});`
`);
});

it("should recommend node compatibility mode when using node builtins and node-compat isn't enabled", async () => {
writeWranglerToml();
fs.writeFileSync(
"index.js",
`
import path from 'path';
console.log(path.join("some/path/to", "a/file.txt"));
export default {}
`
);
let err: Error | undefined;
try {
await runWrangler("publish index.js --dry-run"); // expecting this to throw, as node compatibility isn't enabled
} catch (e) {
err = e as Error;
}
expect(err?.message).toMatch(
`Detected a Node builtin module import while Node compatibility is disabled.\nAdd node_compat = true to your wrangler.toml file to enable Node compatibility.`
);
});

it("should polyfill node builtins when enabled", async () => {
writeWranglerToml();
fs.writeFileSync(
Expand Down
33 changes: 32 additions & 1 deletion packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import assert from "node:assert";
import * as fs from "node:fs";
import { builtinModules } from "node:module";
import * as path from "node:path";
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
Expand All @@ -16,6 +17,33 @@ type BundleResult = {
stop: (() => void) | undefined;
};

/**
* Searches for any uses of node's builtin modules, and throws an error if it
* finds anything. This plugin is only used when nodeCompat is not enabled.
* Supports both regular node builtins, and the new "node:<MODULE>" format.
*/
const checkForNodeBuiltinsPlugin = {
name: "checkForNodeBuiltins",
setup(build: esbuild.PluginBuild) {
build.onResolve(
{
filter: new RegExp(
"^(" +
builtinModules.join("|") +
"|" +
builtinModules.map((module) => "node:" + module).join("|") +
")$"
),
},
() => {
throw new Error(
`Detected a Node builtin module import while Node compatibility is disabled.\nAdd node_compat = true to your wrangler.toml file to enable Node compatibility.`
);
}
);
},
};

/**
* Generate a bundle for the worker identified by the arguments passed in.
*/
Expand Down Expand Up @@ -60,6 +88,7 @@ export async function bundleWorker(
format: entry.format,
rules,
});

const result = await esbuild.build({
...getEntryPoint(entry.file, serveAssetsFromWorker),
bundle: true,
Expand Down Expand Up @@ -87,7 +116,9 @@ export async function bundleWorker(
moduleCollector.plugin,
...(nodeCompat
? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()]
: []),
: // we use checkForNodeBuiltinsPlugin to throw a nicer error
// if we find node builtins when nodeCompat isn't turned on
[checkForNodeBuiltinsPlugin]),
],
...(jsxFactory && { jsxFactory }),
...(jsxFragment && { jsxFragment }),
Expand Down

0 comments on commit 3545e41

Please sign in to comment.