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

node-fetch v2.7.0: punycode is deprecated in 'whatwg-url' and 'tr46' #177

Open
spetrac opened this issue Jan 3, 2024 · 9 comments
Open

Comments

@spetrac
Copy link

spetrac commented Jan 3, 2024

Currently there are several issues raised for node-fetch v2.x (#1793, #1794, #1797) regarding the deprecation of punycode in whatwg-url up until v12.x (#261). There are over 5000 dependents on this module and using any of those packages outputs a deprecation warning regarding punycode.

const nodeFetch = require('node-fetch')
const realFetch = nodeFetch.default || nodeFetch

exports.Headers = nodeFetch.Headers
exports.Request = nodeFetch.Request
exports.Response = nodeFetch.Response

As a solution I would suggest requiring the node-fetch module only when there is not already a fetch in the global scope (which was already introduced in node v17.5.0 and is stable since v21.0.0).

const nodeFetch = global.fetch || require('node-fetch') 
const realFetch = nodeFetch.default || nodeFetch 
exports.Headers = nodeFetch.Headers || global.Headers
exports.Request = nodeFetch.Request || global.Request
exports.Response = nodeFetch.Response || global.Response

With this everything should be backwards compatible and also upwards compatible with newer node versions. The only thing that might not be included is the agent options for the fetch method to inject a node http agent, but this feature of node-fetch is not a fetch standard anyways.

@iegik
Copy link

iegik commented Jun 14, 2024

This because whatwg-fetch replaces global.fetch!! #184

@pfoerdie
Copy link

I don't think this is relevant here, because the rollup.config.js script, which uses whatwg-fetch, is meant to generate the browser version of the polyfill/ponyfill and is only used in the build step.

/* Rollup creates the browser version of the polyfill and ponyfill. */

This issue is not about the browser version, but about warning showing in the nodejs environment. And I think the problem and its solution is already described pretty accurate.
It seems that the author of this package does not maintain it any further, because the last update is 1 year old and none of the issues was addressed ever since. I would advice library authors to find alternatives to this package for that reason.

@pfoerdie
Copy link

BTW, a quickfix for this issue is to create an override in the package.json for whatwg-url:

  "overrides": {
    "[email protected]": {
      "whatwg-url": "14.x"
    }
  },

@mo
Copy link

mo commented Sep 17, 2024

node 22 is soon LTS it would be nice to upgrade node-fetch to 3.x to get rid of the punycode warning

@mo
Copy link

mo commented Sep 17, 2024

running with NODE_OPTIONS="--disable-warning DEP0040" will disable the deprecation warning for punycode

@spetrac
Copy link
Author

spetrac commented Sep 18, 2024

@mo I think disabling warnings is never a good solution, because those warning are not irrelevant, when any node version could finally deprecate the punycode module. There are more practical solutions, like overriding the whatwg-url dependency.

Regardless of the author not actively maintaining this package any more and ignoring any pull requests, upgrading this to node-fetch 3.x will also result in loosing support for older node versions. That means, the cross-fetch module would have to increase its version to 5.0.0, because this is a major change. This also means any depending libraries would have to update their dependencies manually which they could have done a long time ago to move away from this module. In modern environments it is not really needed anyways, because fetch is already adopted and dependencies like node-fetch 3.x or undici-fetch, which is nodes internal fetch library, could be used directly without the need for cross-fetch. If you know your environment, just pick the best fetch solution, and if you do not know your environment, using cross-fetch will not reduce your bundle-size if you do not need it, so just pick any fitting fetch-library directly. This approach will result in much more stable runtime behavior, because the fetch libraries are somewhat different actually, like http-Agent support in node-fetch or undicis dispatcher alternative. If you really don't want to override the global fetch if it already exists, just do it by yourself with global.fetch ??= require('node-fetch') or a similar way.

@mo
Copy link

mo commented Sep 18, 2024

@spetrac I have written a CLI tool and now every time it's used it prints a punycode warning which is really annoying. I get the warning because of this dependency chain (also also several other dependency chains like for example I use "jsdom" and "sentry" which both have transitive deps on old whatwg-url versions):
├─┬ [email protected]
│ └─┬ [email protected]
│ └─┬ [email protected]
│ └─┬ [email protected]
│ └── [email protected]
I've already upgraded all my direct dependencies and that doesn't fix the issue. I don't use cross-fetch or node-fetch myself, I only get them via transitive deps.

I tried adding an override in my top-level package.json like this:
"overrides": { "whatwg-url": "14.x" }

...and while "npm ls whatwg-url" reports 14.0.0 for all instances of whatwg-url I still get the punycode error.

Suppressing warnings in general is of course bad and should not be done at all in libraries, but in my CLI application it's the only option to keep my app usable. Since I have no control of my transitive deps I cannot get rid of the warning in any other way (considering that the package.json "overrides" didn't work for me). And letting my end-users see the warning is of course even more pointless since they certainly cannot fix it, they are not even developers.

@spetrac
Copy link
Author

spetrac commented Sep 18, 2024

I know a warning is pretty annoying for end-users, but it would be more annoying if the program crashes on their machine and they see no warning at all.

If you can confirm that no whatwg-url 5.x-12.x is used anywhere, I would suspect that this might not be the only module with the punycode issue. I know its tedious, but there is no help but dig deeper until all punycode issues are resolved.

BTW, in the current version of puppeteer 23.x they already dropped the cross-fetch dependency.

@mo
Copy link

mo commented Sep 18, 2024

@spetrac ah yes, you're right... I enabled --trace-warnings now and ran with the whatwg-url overrides in package.json and then I found this depency chain:
└─┬ [email protected]
└─┬ [email protected]
└── [email protected]
...where psl package uses punycode as well and they havn't released any version without punycode yet (there is a fix but no maintainer for the package). It seems tough-cookie released 5.0 where they dropped "psl" as a dependency though. With these overrides I get no deprecation warning:
"overrides": { "whatwg-url": "14.x", "tough-cookie": "5.x" }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants