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

refactor: use esbuild-plugins-node-modules-polyfill #5209

Merged
merged 1 commit into from
Mar 18, 2024
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
27 changes: 27 additions & 0 deletions .changeset/hungry-ladybugs-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
"wrangler": major
---

refactor: use `esbuild-plugins-node-modules-polyfill`

Replaces `@esbuild-plugins/node-globals-polyfill` & `@esbuild-plugins/node-modules-polyfill` with the up-to-date & maintained `esbuild-plugins-node-modules-polyfill`

The `esbuild-plugins` repository actually points towards using `esbuild-plugin-polyfill-node` instead
https://github.com/remorses/esbuild-plugins/blob/373b44902ad3e669f7359c857de09a930ce1ce90/README.md?plain=1#L15-L16

But the Remix repo (see https://github.com/remix-run/remix/pull/5274) tried this and found some regressions.
So they chose to go for @imranbarbhuiya's `esbuild-plugins-node-modules-polyfill` instead (see https://github.com/remix-run/remix/pull/6562), which is an up-to-date and well maintained alternative.

Users should no longer see the following deprecation warnings when installing Wrangler:

```sh
npm WARN deprecated rollup-plugin-inject@3.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-inject.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
```

Resolves https://github.com/cloudflare/workers-sdk/issues/1232

**Possible Breaking Change:**
Since we are swapping out the entire polyfill library for a new one, there is a chance that projects using `node_compat` will experience regressions when trying to deploy.

If you have such a Worker, ensure that you test it carefully before deploying when migrating from Wrangler v3 to Wrangler v4.
3 changes: 1 addition & 2 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,10 @@
},
"dependencies": {
"@cloudflare/kv-asset-handler": "workspace:*",
"@esbuild-plugins/node-globals-polyfill": "^0.2.3",
"@esbuild-plugins/node-modules-polyfill": "^0.2.2",
"blake3-wasm": "^2.1.5",
"chokidar": "^3.5.3",
"esbuild": "0.18.20",
"esbuild-plugins-node-modules-polyfill": "^1.6.3",
"miniflare": "workspace:*",
"nanoid": "^3.3.3",
"path-to-regexp": "^6.2.0",
Expand Down
3 changes: 1 addition & 2 deletions packages/wrangler/scripts/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ export const EXTERNAL_DEPENDENCIES = [
// todo - bundle miniflare too
"selfsigned",
"source-map",
"@esbuild-plugins/node-globals-polyfill",
"@esbuild-plugins/node-modules-polyfill",
"esbuild-plugins-node-modules-polyfill",
"chokidar",
// @cloudflare/workers-types is an optional peer dependency of wrangler, so users can
// get the types by installing the package (to what version they prefer) themselves
Expand Down
16 changes: 13 additions & 3 deletions packages/wrangler/src/deployment-bundle/bundle.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
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";
import * as esbuild from "esbuild";
import { nodeModulesPolyfillPlugin } from "esbuild-plugins-node-modules-polyfill";
import { UserError } from "../errors";
import { getBasePath, getWranglerTmpDir } from "../paths";
import { applyMiddlewareLoaderFacade } from "./apply-middleware";
Expand Down Expand Up @@ -34,6 +34,11 @@ export const COMMON_ESBUILD_OPTIONS = {
// build conditions used by esbuild, and when resolving custom `import` calls
export const BUILD_CONDITIONS = ["workerd", "worker", "browser"];

const modulesToPolyfill: Record<string, boolean | "empty"> = {
...Object.fromEntries(builtinModules.map((m) => [m, true])),
net: "empty",
};

/**
* Information about Wrangler's bundling process that needs passed through
* for DevTools sourcemap transformation
Expand Down Expand Up @@ -348,7 +353,12 @@ export async function bundleWorker(
plugins: [
moduleCollector.plugin,
...(legacyNodeCompat
? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()]
? [
nodeModulesPolyfillPlugin({
globals: { Buffer: true, process: true },
modules: modulesToPolyfill,
}),
]
: []),
nodejsCompatPlugin(!!nodejsCompat),
cloudflareInternalPlugin,
Expand Down
76 changes: 15 additions & 61 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion templates/worker-aws/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"@aws-sdk/client-sqs": "^3.82.0"
},
"devDependencies": {
"@esbuild-plugins/node-modules-polyfill": "0.1.4",
"esbuild-plugins-node-modules-polyfill": "^1.6.3",
"worktop.build": "0.0.5",
"wrangler": "^3.0.0"
}
Expand Down
4 changes: 2 additions & 2 deletions templates/worker-aws/worktop.config.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { define } from 'worktop.build';
import { NodeModulesPolyfillPlugin } from '@esbuild-plugins/node-modules-polyfill';
import { nodeModulesPolyfillPlugin } from 'esbuild-plugins-node-modules-polyfill';

// @ts-ignore
export default define({
modify(config) {
config.plugins = config.plugins || [];
config.plugins.push(NodeModulesPolyfillPlugin());
config.plugins.push(nodeModulesPolyfillPlugin());
},
});
Loading