lupomontero / psl

JavaScript domain name parser based on the Public Suffix List
https://www.npmjs.org/package/psl
MIT License
388 stars 80 forks source link

Dependency on deprecated Node "punycode" module #296

Closed Macil closed 10 months ago

Macil commented 2 years ago

The project calls require('punycode'), using the Node punycode module which has been deprecated since v7 (2016). The Node docs now recommend using punycode.js in place of it.

This also means that when this library is used in a project using webpack, webpack must be manually configured to load punycode.js as a replacement for the punycode module because webpack since v5 (2020) no longer defaults to including shims for Node APIs like punycode.

It would be helpful if this library switched to using punycode.js instead of the Node punycode module.

wegry commented 2 years ago

For browser targets using psl, punycode.js's readme has this wrinkle in it:

The current version supports recent versions of Node.js only. It provides a CommonJS module and an ES6 module. For the old version that offers the same functionality with broader support, including Rhino, Ringo, Narwhal, and web browsers, see v1.4.1.

Macil commented 2 years ago

I believe that part of the readme about browser support is just referring to that they started using ES2015isms like const and discontinued their UMD build which could work in pre-ES2015 browsers without a bundler. I'm using Punycode.js in browsers fine through Webpack right now, and Punycode.js's ESM build even works directly as-is in browsers. (I think Punycode.js's readme is misleadingly worded and I've sent a PR to fix that: https://github.com/mathiasbynens/punycode.js/pull/118.)

This library psl is already in CommonJS format and requires a bundler to be used within a browser, so adding a dependency on Punycode.js v2 would not cause a regression for people targeting (ES2015-supporting non-ancient) browsers specifically. But it would cause an issue for people not transpiling their code like with Babel and targeting ancient runtimes that don't support ES6 features like const such as Rhino. It might be good for psl to do a major version update if it adds a dependency on Punycode.js v2 then.

lupomontero commented 2 years ago

Many thanks for reporting this @Macil :star:

I have opened a PR with the suggested change (see #298). It would be amazing if you could try it before I merge it :wink:

npm i lupomontero/psl#punycode.js

But it would cause an issue for people not transpiling their code like with Babel and targeting ancient runtimes [...]

This repo also provides browserified and minified versions of psl intended for old school browser usage (via <script>). This should still work on old browsers.

I recently added automated cross-browser testing (thanks to a free account provided by @browserstack) and will be adding more browser/os combinations soon to make sure we provide enough coverage for old browsers (and Node versions).

wegry commented 2 years ago

@lupomontero after using yarn add psl@https://github.com/lupomontero/psl#punycode.js (with yarn@3) and removing the corresponding webpack@5 resolve.fallback.punycode, this branch appears to work as I'd expect.

lupomontero commented 2 years ago

Nice one @wegry :muscle:

brycefranzen commented 1 year ago

When can we get this merged?

gajus commented 1 year ago

@lupomontero any chance of getting this out?

ddolcimascolo commented 1 year ago

Hi guys, any chance we can get this merged now that Node 21 rolls out?

Cheers, David

luckydonald commented 9 months ago

Will this be in a release anytime soon?