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

API for temporarily supressing experimental warnings #55505

Open
joyeecheung opened this issue Oct 23, 2024 · 3 comments
Open

API for temporarily supressing experimental warnings #55505

joyeecheung opened this issue Oct 23, 2024 · 3 comments
Labels
experimental Issues and PRs related to experimental features. feature request Issues that request new features to be added to Node.js.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Oct 23, 2024

Spinned off from #55417 to discuss about this more generally.

While it can be abused and lead to unwanted issues, I think CLI tools tend to inevitably want to do it for a cleaner output and will find their way to do it already. One example that I've found previously was how Yarn does it by temporarily monkey-patching process.emit

https://github.com/yarnpkg/berry/blob/031b5da1dc8e459e844efda137b2f00d7cdc9dda/packages/yarnpkg-pnp/sources/loader/applyPatch.ts#L304-L325

So I think we might as well provide a proper API for this instead of having users resort to monkey-patching.

We could either just have something quick and simple like process.noExperimentalWarning toggle that gets checked before the warning is emitted (similar to process.noDeprecation):

// Temporarily turn off the warning
process.noExperimentalWarning = true;
// ...use the experimental API
// process.noExperimentalWarning = false;

Or some API that toggles specific warnings (this requires assigning code to experiments, which we currently do not have):

process.silenceExperimentalWarning && process.silenceExperimentalWarning('EXP1', true);
// ...use the experimental API
process.silenceExperimentalWarning && process.silenceExperimentalWarnings('EXP1', false);

Or some API that takes a function and run it without emitting warnings (may be a bit awkward for async APIs, but ensures that users don't forget to toggle it back):

process.runWithoutExperimentalWarning(() => {
   // ...use the experimental API
}, 'EXP1');  // if the experiment code is not passed, silence all experimental warnings

For the ones that take codes we could also merge it with deprecation warning handling and just have e.g. process.silenceWarning()/process.runWithoutWarning() that also accepts DEP codes.

More ideas are welcomed, too.

@avivkeller avivkeller added feature request Issues that request new features to be added to Node.js. experimental Issues and PRs related to experimental features. labels Oct 23, 2024
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Oct 23, 2024
@avivkeller
Copy link
Member

avivkeller commented Oct 23, 2024

WDYT about process.noExperimental for parity with doDeprecation, and you can set it to things like:

true // disable all
"EXPZZ" // disable "EXPZZ"
["EXPZZ", "EXPYY"] // disable both

Even more so, we could condense the no-deprecation and no-experimental into a single disable-warnings functionality, where codes for both can be specified.

E.G.

process.noWarnings = "EXP" // All experimental
process.noWarnings = "DEPZZ" // DEPZZ
process.noWarnings = [<codes>] // <codes> are disabled

Etc

@joyeecheung
Copy link
Member Author

A polymorphic property can be more confusing, it's better to keep the type consistent.

@DavidAnson
Copy link

As an author of a CommonJS tool that probes dynamic module names of unknown pedigree to require/import() them appropriately, I found my way here after trying to enable Node 23 in CI. :)

I appreciate the work on enabling require(esm) and offer two thoughts for consideration:

  1. The probing scenario may be very niche, but if it's used by others, I hope the new behavior is backward-compatible in its failure mode. (What I've read in the relevant discussions suggests it is, thank you.)
  2. As it relates to this issue specifically, my vote is for any exclusion API to force (or at least allow) the caller to specify which experimental warning is being suppressed. This has 3 benefits:
    1. It's clear to future readers of the code what, specifically, is being ignored.
    2. It prevents accidental ignoring of other, perhaps more serious, issues under an overly-broad umbrella.
    3. It enables tooling (like ESLint) to eventually warn on outdated suppressions (for example when Node 23 goes out of support).

Thank you for your time and consideration.

PS - If interesting for any reason, this is the relevant code from markdownlint-cli2:
https://github.com/DavidAnson/markdownlint-cli2/blob/da711fa8b30421506086eb2dc7ea0a6c3cb27b9f/markdownlint-cli2.js#L88-L120

PPS - If there are better ways to implement this behavior, I'm happy to learn of them. There are some extra levels of indirection at play in the code above due to it needing to support dynamic module import after being Webpack'd and also needing to run in Node, browser, and VS Code (WebWorker) context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Issues and PRs related to experimental features. feature request Issues that request new features to be added to Node.js.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

3 participants