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

Dependency on deprecated Node "punycode" module #296

Closed
Macil opened this issue Jun 15, 2022 · 9 comments · Fixed by #298
Closed

Dependency on deprecated Node "punycode" module #296

Macil opened this issue Jun 15, 2022 · 9 comments · Fixed by #298
Assignees

Comments

@Macil
Copy link

Macil commented Jun 15, 2022

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
Copy link

wegry commented Jun 15, 2022

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
Copy link
Author

Macil commented Jun 16, 2022

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: mathiasbynens/punycode.js#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
Copy link
Owner

Many thanks for reporting this @Macil

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

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
Copy link

wegry commented Jul 5, 2022

@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
Copy link
Owner

Nice one @wegry 💪

@brycefranzen
Copy link

When can we get this merged?

kevinoid added a commit to kevinoid/travis-status that referenced this issue Oct 21, 2023
In Node.js 21, `require('punycode')` causes `DeprecationWarning` to be
thrown.  Since `psl` (dep of `tough-cookie`, dep of `request`) requires
punycode, this causes tests to fail.  Disable until
lupomontero/psl#296 is fixed.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@gajus
Copy link

gajus commented Oct 30, 2023

@lupomontero any chance of getting this out?

@ddolcimascolo
Copy link

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

Cheers,
David

@luckydonald
Copy link

Will this be in a release anytime soon?

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