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

Closed
coreyfarrell opened this issue Dec 5, 2019 · 36 comments · Fixed by #50661
Closed

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

coreyfarrell opened this issue Dec 5, 2019 · 36 comments · Fixed by #50661
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

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

@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

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

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.
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

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

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

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).

@karlhorky
Copy link
Contributor

karlhorky commented Apr 8, 2023

Ok, opened an issue here:

@Cojad
Copy link

Cojad commented Jul 4, 2023

I want a solution which is simple yet easy to port to any project using fetch. So I shorten it into just 2 lines of code in my entry script.

// Using only 2 line at begining of your entry script(ex: index.js).
// Or any place before using fetch to suppress warnings for ExperimentalWarning(just once before calling fetch).

// inspired by 
// https://github.com/nodejs/node/issues/30810#issuecomment-1433950987
const { emit: originalEmit } = process;
process.emit = (event, error) => event === 'warning' && error.name === 'ExperimentalWarning' ? false : originalEmit.apply(process, arguments);

@jeffs
Copy link

jeffs commented Jul 4, 2023

const { emit: originalEmit } = process;
process.emit = (event, error) => event === 'warning' && error.name === 'ExperimentalWarning' ? false : originalEmit.apply(process, arguments);

That arguments object isn't from your wrapper function; it's a free variable. The arguments object isn't defined by fat arrow functions. You're accidentally passing originalEmit an unrelated arguments object that Node apparently puts in the prelude for scripts (though not for the REPL), and dropping your actual parameters. Consider using a rest parameter instead. See also MDN.

Maybe you could share a GitHub repo or NPM package complete with unit and integration tests. It would be nice if your solution also type-checked properly. I'm sure folks would be happy to review your code.

nodejs-github-bot pushed a commit that referenced this issue Nov 21, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
targos pushed a commit that referenced this issue Nov 23, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
martenrichter pushed a commit to martenrichter/node that referenced this issue Nov 26, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#50661
Fixes: nodejs#30810
Fixes: nodejs#47478
Fixes: nodejs#46862
Fixes: nodejs#40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
lucshi pushed a commit to lucshi/node that referenced this issue Nov 27, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#50661
Fixes: nodejs#30810
Fixes: nodejs#47478
Fixes: nodejs#46862
Fixes: nodejs#40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
RafaelGSS pushed a commit that referenced this issue Nov 29, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
RafaelGSS pushed a commit that referenced this issue Nov 30, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
UlisesGascon pushed a commit that referenced this issue Dec 11, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
UlisesGascon pushed a commit that referenced this issue Dec 15, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
UlisesGascon pushed a commit that referenced this issue Dec 19, 2023
Co-authored-by: Geoffrey Booth <webadmin@geoffreybooth.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: #50661
Fixes: #30810
Fixes: #47478
Fixes: #46862
Fixes: #40940
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
liudonghua123 added a commit to liudonghua123/node-sea that referenced this issue Jan 16, 2024
@liudonghua123
Copy link
Contributor

I add a shebang like this to make the bin script working without showing the warnings. And it works for windows too.

#!/usr/bin/env node --no-warnings=ExperimentalWarning

@egasimus
Copy link

egasimus commented Jan 16, 2024

I add a shebang like this to make the bin script working without showing the warnings. And it works for windows too.

#!/usr/bin/env node --no-warnings=ExperimentalWarning

On my Linux box, this needs to be:

#!/usr/bin/env -S node --no-warnings=ExperimentalWarning

The -S flag is needed to split the command line arguments. Without it, the script just hangs, doesn't even start. Does macOS env support the -S flag?

I saw somebody do an executable Dockerfile with env -S recently - but does it work out of the box on all platforms today?

The other thing you gotta keep in mind when starting a script with a shebang is that it becomes unimportable in <script type="module">. Which is mostly fine for a CLI entrypoint unless you want to write an entire isomorphic app in 1 file, in which case, no dice 😅

@liudonghua123
Copy link
Contributor

@egasimus Yes, I also find this option is necessary when running on linux.

see also liudonghua123/node-sea@04fa6ec.

@egasimus
Copy link

@liudonghua123 Yes. Does it still work on Mac and Windows when you add that option?

@liudonghua123
Copy link
Contributor

@liudonghua123 Yes. Does it still work on Mac and Windows when you add that option?

It works on windows and linux, I have not tested on macos yet.

@rauschma
Copy link

Also works on macOS! I just tested it.

@Sv443
Copy link

Sv443 commented Jun 1, 2024

Hi, does anyone know a way of doing this with just environment variables? In my case I have no access to the args of the node binary.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 1, 2024

does anyone know a way of doing this with just environment variables?

$ NODE_OPTIONS=--disable-warning=ExperimentalWarning node -e 'require("node:vm").measureMemory()'

qianl15 added a commit to dbos-inc/dbos-transact-ts that referenced this issue Jul 28, 2024
According to nodejs/node#30810
We need to use `#!/usr/bin/env -S node
--no-warnings=ExperimentalWarning` on Linux. Otherwise it hangs. This
flag also works on other OS environments.
@o0101
Copy link

o0101 commented Aug 1, 2024

Can node process set its own process.env.NODE_OPTIONS ? Or will that not have effect after startup

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
None yet