*DUPLICATE* [wrangler] Use esbuild-plugins-node-modules-polyfill for Node.js polyfills instead of @esbuild-plugins

[rnmeow]

What this PR solves / how to test:

Removes dependency @esbuild-plugins/node-globals-polyfill and @esbuild-plugins/node-modules-polyfill since they're both out-of-date.

Adds dependency esbuild-plugin-polyfill-node for support of Node.js polyfills.

This is probably not needed to test, without breaking changes, I think.

Author has addressed the following:

• Tests
  • Included
  • Not necessary because: Unrelated.
• Changeset (Changeset guidelines)
  • Included
  • Not necessary because:
• Associated docs
  • Issue(s)/PR(s):
  • Not necessary because: Unrelated.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7488a42

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
wrangler	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[petebacondarwin]

Let me see if I can get the CI checks running on this...

[github-actions]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7783226811/npm-package-wrangler-4902

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/4902/npm-package-wrangler-4902

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7783226811/npm-package-wrangler-4902 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7783226811/npm-package-create-cloudflare-4902 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7783226811/npm-package-miniflare-4902

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7783226811/npm-package-cloudflare-pages-shared-4902

Note that these links will no longer work once the GitHub Actions artifact expires.

---

[email protected] includes the following runtime dependencies:

Package	Constraint	Resolved
miniflare	workspace:*	3.20240129.0
workerd	1.20240129.0	1.20240129.0
workerd --version	1.20240129.0	2024-01-29

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

[petebacondarwin]

I can't push a fixup commit to your PR as you did not allow contributors to update your branch.
But you need to fix the formatting with this diff:

diff --git a/packages/wrangler/src/deployment-bundle/bundle.ts b/packages/wrangler/src/deployment-bundle/bundle.ts
index dab86a06..cd7b0db9 100644
--- a/packages/wrangler/src/deployment-bundle/bundle.ts
+++ b/packages/wrangler/src/deployment-bundle/bundle.ts
@@ -325,12 +325,14 @@ export async function bundleWorker(
                plugins: [
                        moduleCollector.plugin,
                        ...(legacyNodeCompat
-                               ? [nodeModulesPolyfillPlugin({
-                                               globals: {
-                                                       process: true,
-                                                       Buffer: true,
-                                               },
-                                       })]
+                               ? [
+                                               nodeModulesPolyfillPlugin({
+                                                       globals: {
+                                                               process: true,
+                                                               Buffer: true,
+                                                       },
+                                               }),
+                                 ]
                                : []),
                        ...(nodejsCompat ? [nodejsCompatPlugin] : []),
                        cloudflareInternalPlugin,

You can fix this via the command line by running pnpm fix in the root of the monorepo.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3cb2a8a) 70.64% compared to head (f7c91ef) 70.61%.
Report is 8 commits behind head on main.

❗ Current head f7c91ef differs from pull request most recent head 7488a42. Consider uploading reports for the commit 7488a42 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4902      +/-   ##
==========================================
- Coverage   70.64%   70.61%   -0.04%     
==========================================
  Files         292      292              
  Lines       15183    15186       +3     
  Branches     3863     3865       +2     
==========================================
- Hits        10726    10723       -3     
- Misses       4457     4463       +6     

Files	Coverage Δ
packages/wrangler/src/deployment-bundle/bundle.ts	87.82% <100.00%> (-0.11%)	⬇️

... and 3 files with indirect coverage changes

[petebacondarwin]

I'm a bit concerned about these issues with the proposed package:

• Globals are always included for commonjs modules cyco130/esbuild-plugin-polyfill-node#8
• Error when using with Vite cyco130/esbuild-plugin-polyfill-node#13
• Doesn't support ESM cyco130/esbuild-plugin-polyfill-node#11

I think we need to check that these do not impair Wrangler before we can merge this.

[rnmeow]

I'm a bit concerned about these issues with the proposed package:
...
I think we need to check that these do not impair Wrangler before we can merge this.

At a glance, the problems seem to appear at Vite-side and old Node.js runtimes. However, to examine a little bit more will be safer (especially for the users).

[mrbbot]

I believe this is duplicate of #3832. Per #3832 (comment), we believe this would be a breaking change and as such need to wait until the next major release before we can land this. In the meantime, we recommend using Custom builds if you need additional polyfills.

[rnmeow]

I believe this is duplicate of #3832. Per #3832 (comment) ...

Closing.

PS. It's surprising that to deprecate the packages is such a breaking change.