josephtzeng / hocon-parser

A HOCON Parser for Node.js
ISC License
31 stars 10 forks source link

Upgrade `node-fetch` dependency #35

Closed eliellis closed 5 months ago

eliellis commented 5 months ago

What

Currently, when requiring @pushcorn/hocon-parser in a script using Node.js 21 or greater, a deprecation warning regarding punycode is thrown:

CleanShot 2024-06-19 at 23 08 38@2x

Why

This is due to @pushcorn/hocon-parser directly depending on node-fetch@2.7.0, which itself depends on whatwg-url@^5.0.0, which depends on the now-deprecated built-in punycode module.

Doing some investigation, it seems like the correct course of action to solve this would be (in a perfect world) for this package to simply update its dependency on node-fetch to version 3, given that this module mandates a minimum Node.js version of 14 or greater already...

However node-fetch v3 and greater is ESM only, which opens up a whole bag of worms around compatibility with existing users of the library. I'm not sure what the maintainers want to do here, but I am opening this issue as a means to track the deprecation warning because of this transient dependency. Eventually a major release of Node.js will remove the punycode module altogether and at that point, node-fetch@2.7.0 will no longer function with those major versions of Node.js, which, in turn, means that this library will not function with those higher versions of Node.js either. πŸ˜”

Ideally, the best course of action would be to find a CJS (and possibly additionally, an ESM) compatible way to resolve the issue.

Possible Solutions

  1. Upgrade node-fetch v3, make this package ESM-only, and bump this package's major version to to signify the breaking change
    • This one is particularly not ideal, as this means importing this package becomes trouble for CJS users
  2. Use Node.js' own fetch module directly, remove the node-fetch dependency altogether, update the engines for this package to nodejs >= 21 and bump this package's major version to signify the breaking change
  3. Find an alternative for node-fetch that does not suffer from the deprecation, doesn't require any changes to the engines field, and wouldn't require very many changes to this code-base
    • (personally) this feels like the best course of action, given its minimal overhead for the maintainer, and ideally doesn't break anything for end-users
    • one CJS fetch replacement that I am aware of is ofetch
josephtzeng commented 5 months ago

Thanks for reporting the issue. I've replaced node-fetch with rock-req, which is smaller. Please try the latest version.

eliellis commented 5 months ago

Wow! Thank you for such a quick response. I really appreciate it. Latest version yields no warning. πŸ‘