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

debug causes npm to always issue an experimental ESM warning in node 23 #975

Open
ljharb opened this issue Oct 16, 2024 · 17 comments · May be fixed by #977
Open

debug causes npm to always issue an experimental ESM warning in node 23 #975

ljharb opened this issue Oct 16, 2024 · 17 comments · May be fixed by #977

Comments

@ljharb
Copy link

ljharb commented Oct 16, 2024

Specifically,

const supportsColor = require('supports-color');
is triggering it. See also nodejs/node#55338 (comment) and nodejs/node#55397.

I'm not sure the best solution here. One is to use optional peer deps to force supports-color to be < 9; another is to do monkeypatching shenanigans around that require to suppress the experimental warning; I'm sure there's more.

@joyeecheung
Copy link

joyeecheung commented Oct 16, 2024

The warning can also be temporarily disabled using process.noDeprecation = true (it's kind of an abuse because it's an experimental warning, not a deprecation warning, and it's kind of a bug that it affects the experimental warnings, but hey the bug is out there already). A slightly saner solution can be checking whether process.version starts with v23 (the warning will be restricted to code not coming from node_modules on v22, and will probably be gone by 24).

@MikeMcC399
Copy link

MikeMcC399 commented Oct 20, 2024

[email protected], released in Apr 2021, was the first version to use pure ESM.

This issue affects any project where supports-color >= 9.0.0 (ESM only) is installed together with debug. It is not restricted to npm.

For instance, on Ubuntu 24.04.1 LTS, Node.js 23.0.0, npm 10.9.0, executing the following:

git clone https://github.com/cypress-io/github-action
cd github-action
npm ci
node src/ping-cli.js https://example.cypress.io

not only shows the ExperimentalWarning from npm ci, it also shows the warning from

node src/ping-cli.js https://example.cypress.io

which is not using npm at all:

$ node src/ping-cli.js https://example.cypress.io
pinging url https://example.cypress.io for 30 seconds
(node:5900) ExperimentalWarning: Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

https://github.com/cypress-io/github-action is configured with [email protected], i.e. a pure ESM version.

@ljharb
Copy link
Author

ljharb commented Oct 20, 2024

@MikeMcC399 however, it'd only trigger on projects that require that version, which virtually nobody would do, except in this package's case where it's treated like an implicit optional dep - requiring < 9, and importing any version, wouldn't trigger the warning.

@MikeMcC399
Copy link

Under Node.js v23.1.0 the warning provides additional debugging information which confirms the previous analysis:

$ npm view npm version
(node:13088) ExperimentalWarning: CommonJS module /home/mike/n/lib/node_modules/npm/node_modules/debug/src/node.js is loading ES Module /home/mike/n/lib/node_modules/npm/node_modules/supports-color/index.js using require().
Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
10.9.0

@Qix-
Copy link
Member

Qix- commented Nov 4, 2024

use optional peer deps

This is probably fine; I don't think that was around when I implemented the optional dependency stuff. PR welcome and I'll cut a patch release ASAP (especially if it's within the next 48 hours, otherwise might be after Nov 16th at some point. Some last minute traveling happening for urgent reasons. Ping me here if it falls through the cracks).

@wellwelwel
Copy link

wellwelwel commented Nov 4, 2024

@Qix-, first of all, thanks for your attention to this issue.

I would really appreciate it if you could take a look at npm/cli#7857 (comment), since it would directly implicate this project and, naturally, npm/cli (in a good way).

I can't say what the community thinks about this yet, but it's a solution that seems both user-friendly and easy to implement (locally, everything is practically ready), plus that wouldn't overload the team of any of the involved projects (debug, supports-color, and npm/cli).

@Qix-
Copy link
Member

Qix- commented Nov 20, 2024

@stefee kindly submitted a PR, but the regressions stated here are a bit bothersome.

The only other "correct" alternative is to switch to ESM-only and use import, but I'd imagine that doesn't really solve problems people are facing today.


There's a less-nice alternative. Start without any extended color support, fire off an async import() and then once it completes retroactively enable extended color support. The first few log messages will have the basic colors but switch later on. I don't know how terrible of an idea that is, though.

@jonkoops
Copy link

jonkoops commented Nov 20, 2024

Another option would be to introduce a build system to ship debug as a dual-module package, so both ESM and CJS versions are included. This would allow users seeing this warning to adopt ESM, which would then remove the warning.

@backflip
Copy link

In case people are not following this linked issue: There's a possibility the warning will be hidden in Node 23, too: nodejs/node#55217 (comment)

@Qix-
Copy link
Member

Qix- commented Nov 20, 2024

I'm not super keen on adding a build system, personally. Debug shouldn't need one; it's a trivial package. If every package ends up needing a build system then I'm not sure what that says about the Node.js ecosystem.

Perhaps an unpopular opinion.

@jonkoops
Copy link

Totally agree, I am of the personal opinion that we need to get back to simple packages that are aligned across the ecosystem, just wanted to throw out the option. Since it looks like this warning will be removed from Node.js it might be worth it to just wait this one out, as it might resolve itself.

That said, since all Node.js LTS versions will soon have support for importing ESM from CommonJS it might be worth it to then consider adopting ESM as well. But this is likely an entirely separate discussion.

@ljharb
Copy link
Author

ljharb commented Nov 20, 2024

It’ll be possibly years before all LTS versions support that.

@jonkoops
Copy link

Looks like at least 22.x is getting a backport (see nodejs/node#52697), and I was under the impression that the same would be done for 20.x and even 18.x but I could be mistaken. 18.x will be end-of-life the first half of next year as well.

@Qix-
Copy link
Member

Qix- commented Nov 20, 2024

I guess another way of handling this is to simply vendor in supports-color to the codebase and modify it to be CJS, though as a maintainer of both packages I feel like I'd be stabbing myself in the back.

Feels very "putting a stick in your own bicycle spokes meme".

@jonkoops
Copy link

So at the very least 20.x will have a backport (nodejs/node#52697 (comment)). Might be worth a semver major once 18.x reaches EOL.

@ljharb
Copy link
Author

ljharb commented Nov 21, 2024

Unfortunately the stick was making it ESM-only :-/

@joyeecheung
Copy link

FYI for v22.x the warning would be suppressed when the require() is coming from node_modules.

For v23 we are considering the same but complications apply due to LTS policy nodejs/node#55217 (comment)

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

Successfully merging a pull request may close this issue.

7 participants