The Wayback Machine - https://web.archive.org/web/20221231212049/https://github.com/nodejs/node/issues/30810
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

Ability to suppress warnings by type (or just experimental warnings) #30810

Open
coreyfarrell opened this issue Dec 5, 2019 · 18 comments · May be fixed by #36137
Open

Ability to suppress warnings by type (or just experimental warnings) #30810

coreyfarrell opened this issue Dec 5, 2019 · 18 comments · May be fixed by #36137
Labels
feature request Issues that request new features to be added to Node.js. process Issues and PRs related to the process subsystem.

Comments

@coreyfarrell
Copy link
Member

coreyfarrell commented Dec 5, 2019

Is your feature request related to a problem? Please describe.
I'd like to suppress experimental warnings while still seeing any other errors. In particular when I am using native ES modules I do not want the experimental warning printed for every process, but I do want unrelated warnings to still be printed.

Describe the solution you'd like
Allow --no-warnings to optionally accept an option string such as --no-warnings=type1,type2. Using --no-warnings without any option would continue to disable all warnings. This would allow --no-warnings=ExperimentalWarning to suppress ExperimentalWarning only.

Describe alternatives you've considered
--no-experimental-warnings or a similarly named new flag could be created. This has the drawback that node --no-experimental-warnings on node.js 13.3.0 exit with an error where --no-warnings=ExperimentalWarnings will not currently error (it causes all warnings to be ignored).

In my own repo which uses ES modules I've created suppress-experimental.cjs which gets loaded with NODE_OPTIONS='--require=./suppress-experimental.cjs':

'use strict';

const {emitWarning} = process;

process.emitWarning = (warning, ...args) => {
	if (args[0] === 'ExperimentalWarning') {
		return;
	}

	if (args[0] && typeof args[0] === 'object' && args[0].type === 'ExperimentalWarning') {
		return;
	}

	return emitWarning(warning, ...args);
};

Obviously patching node.js internals like this is undesirable but it accomplishes my goal.

@jasnell jasnell added feature request Issues that request new features to be added to Node.js. process Issues and PRs related to the process subsystem. labels Dec 5, 2019
codeman869 added a commit to codeman869/node that referenced this issue Dec 23, 2019
Adds a command line option to supress experimental warnings. Currently
this cannot be accomplished without supressing all warnings (by using
the --no-warnings option). However, once this option is enabled, a user
can miss out on essential warnings as this supresses all warnings.
This commit adds the --no-experimental-warnings command line option to
allow users to ignore warnings they will expect while still being able
to monitor unexpected warnings.

Fixes: nodejs#30810
@JoshMcCullough
Copy link

JoshMcCullough commented Mar 6, 2020

args[0] would point to a string, since the arguments to emitWarning are warning: string | Error, name?: string, ctor?: Function. Are you sure this code is working correctly?

@coreyfarrell
Copy link
Member Author

coreyfarrell commented Mar 7, 2020

@JoshMcCullough Yes the code works, note that the parameters for my replacement function are (warning, ...args). I can see how that looks odd but it definitely works.

@JoshMcCullough
Copy link

JoshMcCullough commented Mar 9, 2020

I was suggesting that your args[0] will always be a string since it maps to the 2nd parameter of emitWarning, which is (if the types are correct), name?: string. So the second if block would never be entered -- unless I'm totally missing something here.

@coreyfarrell
Copy link
Member Author

coreyfarrell commented Mar 9, 2020

https://nodejs.org/dist/latest/docs/api/process.html#process_process_emitwarning_warning_options shows that process.emitWarning can also take an options object as the second argument. I don't think the ESM warning uses that style call but I still check for it as I want my code to continue working if the ESM warning switches to the options object.

wincent added a commit to wincent/wincent that referenced this issue Apr 8, 2020
In the absence of something like the feature proposed here:

    nodejs/node#30810

Let's just take the heavy-handed approach of suppressing all warnings
unless `--debug` is in effect. Keeps ugly:

    (node:19198) ExperimentalWarning: The ESM module loader is experimental.

out of the console. Will rip this all out once it moves out of
experimental status.
@cjihrig cjihrig linked a pull request Nov 16, 2020 that will close this issue
4 tasks
PoojaDurgad added a commit to PoojaDurgad/node that referenced this issue Dec 23, 2020
Introduce an option `--suppress-warnings`
to silence experimental and deprecation
warnings for specified features

Fixes: nodejs#30810

Co-Authored-By: Cody Deckard <cjdeckard@gmail.com>
@noahehall
Copy link

noahehall commented Feb 12, 2022

this would have been cool, especially for those of us using experimental loader (which I presume will be experimental for quite some time?)

@ChuanyuanLiu
Copy link

ChuanyuanLiu commented Apr 15, 2022

The proposed hack with suppress-experimental.js did not work for me.

It works when I trigger process.emitWarning manually but it appears that NodeJS's call on Experimental Warning did not go through the redefined process.emitWarning.

My cmd arguments are node --experimental-fetch --require=./suppress-experimental.js index.js

@kachkaev
Copy link

kachkaev commented Apr 24, 2022

@ChuanyuanLiu which version of Node are you using? Overriding process.emitWarning as suggested above worked for me on Node 16, but when I tried Node 18, it did not. It indeed feels like (node:43369) ExperimentalWarning: ... comes from some internals of Node, so cannot be silenced.

UPD I found a patch that works both in Node 16 and 18 🎉
https://github.com/yarnpkg/berry/blob/2cf0a8fe3e4d4bd7d4d344245d24a85a45d4c5c9/packages/yarnpkg-pnp/sources/loader/applyPatch.ts#L414-L435

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Apr 24, 2022

A recent change (#42314) converted some of ESMLoader's experimental warning emissions from using process.emitWarning() directly to an internal utility (emitExperimentalWarning()) that uses it under the hood. Since it still uses process.emitWarning() under the hood, if overwriting process.emitWarning() ever worked, that should continue to work.

@kachkaev
Copy link

kachkaev commented Apr 24, 2022

@JakobJingleheimer looks like in Node 18 uses process.emit instead of process.emitWarning. Judging by a recent change in Yarn Berry: yarnpkg/berry#4338 (diff).

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented May 3, 2022

Could you cite a problematic place where that happens? A quick search of the node codebase suggests it's used only sparingly and in very specific scenarios. process.emitWarning does call process.emit():

emitWarning → doEmitWarning → emit

process.nextTick(doEmitWarning, warning);
}
function emitWarningSync(warning) {
process.emit('warning', createWarningObject(warning));
}

but it has done so for ~2 years (according to git history).

I'm not aware of a policy change and I see no code docs recommending such a switch (and if it did happen, it was apparently incomplete). It's quite possible one of the ~100 active maintainers used something different recently, either because it was better suited or because they didn't know process.emitWarning() exists.

P.S. Regarding the original topic of this issue, I don't foresee suppressing certain warnings being supported (and it would take a significant amount of work to facilitate as there is currently no authoritative list of feature names, only strings that happen to be consistent because humans care to check).

@aral
Copy link

aral commented May 26, 2022

UPD I found a patch that works both in Node 16 and 18 tada https://github.com/yarnpkg/berry/blob/2cf0a8fe3e4d4bd7d4d344245d24a85a45d4c5c9/packages/yarnpkg-pnp/sources/loader/applyPatch.ts#L414-L435

And here’s a plain old JavaScript version of it that also silences a few other experimental warnings (JSON modules, and Node.js specifier resolution):

const originalEmit = process.emit;

process.emit = function (name, data, ...args) {
  if (
    name === 'warning' &&
    typeof data === 'object' &&
    data.name === 'ExperimentalWarning' &&
    (data.message.includes('--experimental-loader') ||
      data.message.includes('Custom ESM Loaders is an experimental feature') ||
			data.message.includes('The Node.js specifier resolution flag is experimental') ||
			data.message.includes('Importing JSON modules is an experimental feature'))
  )
    return false

  return originalEmit.apply(process, arguments)
}

@renhiyama
Copy link

renhiyama commented Jul 18, 2022

I happened to use a smaller version since your code isnt compatible if I use env instead of passing through nodejs' args

const originalEmit = process.emit;
  process.emit = function (name, data, ...args) {
    if (
      name === `warning` &&
      typeof data === `object` &&
      data.name === `ExperimentalWarning`
    )
      return false;

    return originalEmit.apply(process, arguments);
  };

Heres the bash script that runs mine:

NODE_OPTIONS=--experimental-vm-modules node test

@renhiyama
Copy link

renhiyama commented Jul 18, 2022

(I bet you can add extra conditions to check env, but hey, your code would be not as performant as before then and will be 0.001% slower than before!)

@Xunnamius
Copy link

Xunnamius commented Sep 20, 2022

The previous solutions didn't work for me in all cases, though process is an EventEmitter so the following coarse-grain one-liner worked for me:

process.removeAllListeners('warning');

Alternatively, a safer more fine-grained approach to only disable a specific type of warning (ExperimentalWarning in this case):

const warningListeners = process.listeners('warning');

if (warningListeners.length != 1) {
  console.warn(
    `expected 1 listener on the process "warning" event, saw ${warningListeners.length}`
  );
}

if (warningListeners[0]) {
  const originalWarningListener = warningListeners[0];
  process.removeAllListeners('warning');

  process.prependListener('warning', (warning) => {
    if (warning.name != 'ExperimentalWarning') {
      originalWarningListener(warning);
    }
  });
}

Eventually I'll publish this as @xunnamius/suppress-warnings with an interface like suppressWarnings(['ExperimentalWarning']) just to make my life easier.

@dword-design
Copy link

dword-design commented Dec 3, 2022

I wrote a package that seems to do it for me atm: suppress-experimental-warnings. CI-tested with many different node versions + operating systems.

@rulatir
Copy link

rulatir commented Dec 18, 2022

Vicious circle here:

  1. if experimental warnings are impossible to be silenced as long as the features remain experimental, users will never dare throw real-life usages at those features
  2. if users never dare throw real-life usages at those features, then those features will never be sufficiently "proven" by real life usage
  3. if those features will never be sufficiently "proven" by real life usage, they will remain experimental forever

Something must yield in order to unstick this deadlock, and this feature request expresses the position that it is the warnings that should yield.

@rulatir
Copy link

rulatir commented Dec 18, 2022

I wrote a package that seems to do it for me atm: suppress-experimental-warnings. CI-tested with many different node versions + operating systems.

With localizations too?

EDIT: just read your code above and indeed it doesn't seem to rely on output parsing. I'll give it a try.

@replete
Copy link

replete commented Dec 31, 2022

I wrote a package that seems to do it for me atm: suppress-experimental-warnings. CI-tested with many different node versions + operating systems.

This seems to work pretty well FYI...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. process Issues and PRs related to the process subsystem.
Projects
Status: In Progress
Morty Proxy This is a proxified and sanitized view of the page, visit original site.