cloudflare / workers-sdk

⛅️ Home to Wrangler, the CLI for Cloudflare Workers®
https://developers.cloudflare.com/workers/
Apache License 2.0
2.72k stars 713 forks source link

🐛 BUG: deprecated dependencies #6189

Open benmccann opened 4 months ago

benmccann commented 4 months ago

Which Cloudflare product(s) does this pertain to?

Wrangler core

What version(s) of the tool(s) are you using?

3.62.0

What version of Node are you using?

No response

What operating system and version are you using?

any

Describe the Bug

Wrangler has deprecated packages in its dependency tree, which causes deprecation warnings for us when doing pnpm install

https://npmgraph.js.org/?q=wrangler

Screenshot from 2024-07-02 11-09-38

It looks like you need to switch from @esbuild-plugins/node-modules-polyfill to esbuild-plugin-polyfill-node

Please provide a link to a minimal reproduction

pnpm install wrangler

Please provide any relevant error logs

 WARN  2 deprecated subdependencies found: rollup-plugin-inject@3.0.2, sourcemap-codec@1.4.8

petebacondarwin commented 4 months ago

This was discussed at some length in https://github.com/cloudflare/workers-sdk/issues/1232. The current plan is to move to a completely new approach to node-compat that uses unenv and built-in run time modules. We don't plan to update these plugins.

benmccann commented 4 months ago

Can we update the plugins in the meantime instead of serving deprecated versions to all of our users?

What's the release timeline for wrangler 4?

benmccann commented 4 months ago

I found this fixed as the last commit in the v4 branch four months ago: https://github.com/cloudflare/workers-sdk/pull/5209. But there have been no further updates to the branch since then

petebacondarwin commented 4 months ago

These are just warnings. Do you actually have a bug in the node_compat feature that would be resolved by changing these dependencies?

benmccann commented 4 months ago

There's no bug and I get that they're just warnings, but I don't want to teach our users to ignore warnings nor do we want to ignore them in our own projects. Users file bugs against us everytime we have deprecation warnings, it causes our audit checks to fail, and it's just not consistent with the quality of our offering to generate warnings for years while telling users to ignore them.

RamIdeas commented 2 months ago

To provide an update here we have a new node compat feature about to launch soon which will become our primary recommendation for node polyfills but we cannot remove the old node compat feature without a breaking change.

Since this issue pertains to warnings caused by deprecated dependencies needed for the old node compat feature, I've applied a breaking change label.

Although, if we wanted to fix this without a breaking change, we could consider vendoring the deprecated packages into our repo.

benmccann commented 2 months ago

Another idea might be to make the implementation pluggable so that users can choose which implementation to use without pulling in dependencies for the implementation(s) they're not using. That would still be a breaking change, but a pretty small one that would be easy enough to handle while upgrading.

irvinebroque commented 2 months ago

Vendoring makes sense to me

DominiqueComte commented 2 days ago

after reading #1232 and #3612 this bug should have the "Wrangler v4" milestone, right ?