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.

Sign up for GitHub

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

Jump to bottom

make all experimental warning messages consistent #30803

Open
pd4d10 opened this issue Dec 5, 2019 · 5 comments · May be fixed by #53704
Open

make all experimental warning messages consistent #30803

pd4d10 opened this issue Dec 5, 2019 · 5 comments · May be fixed by #53704
Labels
experimental Issues and PRs related to experimental features.

Comments

@pd4d10
Copy link
Contributor

pd4d10 commented Dec 5, 2019 •
edited
Loading

Some experimental warning messages' contents are not consistent, for example:

(node:70168) ExperimentalWarning: The ESM module loader is experimental.
(node:70372) ExperimentalWarning: Conditional exports is an experimental feature. This feature could change at any time

Shall we make them consistent? If so I'm willing to submit a PR to solve this.

There are two general methods to emit experimental warnings. I think we can make use of them.

JavaScript version

node/lib/internal/util.js

Lines 170 to 176 in 1549c8e

function emitExperimentalWarning(feature) {
if (experimentalWarnings.has(feature)) return;
const msg = `${feature} is an experimental feature. This feature could ` +
'change at any time';
experimentalWarnings.add(feature);
process.emitWarning(msg, 'ExperimentalWarning');
}

C++ version (introduced in #30617)

node/src/node_process_events.cc

Lines 102 to 112 in 1549c8e

Maybe<bool> ProcessEmitExperimentalWarning(Environment* env,
const char* warning) {
if (experimental_warnings.find(warning) != experimental_warnings.end())
return Nothing<bool>();
experimental_warnings.insert(warning);
std::string message(warning);
message.append(
" is an experimental feature. This feature could change at any time");
return ProcessEmitWarningGeneric(env, message.c_str(), "ExperimentalWarning");
}

@pd4d10 pd4d10 changed the title make all experimental warning messages consistent Dec 5, 2019
@pd4d10
Copy link
Contributor Author

pd4d10 commented Dec 5, 2019

/cc @guybedford
@guybedford
Copy link
Contributor

We've had the The ESM Loader is experimental message for long enough that I think this one is fine to leave as-is until we remove it.

If there are other inconsistencies though we should change them.
@pd4d10
Copy link
Contributor Author

pd4d10 commented Dec 5, 2019

OK, get it. Found these flags. Seems not related to module.

(node:70168) ExperimentalWarning: Policies are experimental. # --experimental-policy
(node:70168) ExperimentalWarning: The --frozen-intrinsics flag is experimental # --frozen-intrinsics
@pd4d10 pd4d10 changed the title module: make all experimental warning messages consistent Dec 5, 2019
@guybedford
Copy link
Contributor

Those both seem worth doing to me. The added benefit is ensuring that the code examples follow the correct patterns for future reference.
@guybedford
Copy link
Contributor

(//cc @bmeck)
@targos targos added the experimental Issues and PRs related to experimental features. label Dec 26, 2020
@RedYetiDev RedYetiDev linked a pull request Jul 2, 2024 that will close this issue
Open
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.
3 participants
@guybedford @targos @pd4d10