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

How to prevent ExperimentalWarning on queueMicrotask feature detection in library code? #25592

Closed
zloirock opened this issue Jan 20, 2019 · 8 comments
Labels
experimental Issues and PRs related to experimental features.

Comments

@zloirock
Copy link

core-js@3 contains queueMicrotask polyfill.

queueMicrotask added to Node.js 11 as an experimental feature.

On getting queueMicrotask global property on Node.js 11, emitted an ExperimentalWarning. Without a special handler, this warning pollutes the global output.

Even this method not used, core-js detect it by getting this property. So even import of core-js@3 pollutes the global output by this warning.

core-js is a quite popular library, so after updating to stabe core-js@3, pollution of the global output will cause too many problems.

Any ideas what should I do for preventing this problem? process.on('warning', ..)? But it does not look like a solution for a library.

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

There isn't anything you can do to prevent just a single occurrence of a warning. I would say that in this case the experimental warning is doing exactly what it should. People are able to experiment with queueMicrotask, but it prevents people from shipping libraries and such using it until the warning is removed.

@devsnek devsnek added the experimental Issues and PRs related to experimental features. label Jan 20, 2019
@zloirock
Copy link
Author

Seems you don't understand that core-js is a polyfill project. A serious part of core-js features is experimental features, for example, ES proposals. They were implemented in V8 -> Node.js. But, before this issue, any other features does not cause uncatchable warnings. So should I hide all warnings by handling process.on('warning', ..) or leave with this problem millions of projects? Or another solution?

@cjihrig
Copy link
Contributor

cjihrig commented Jan 20, 2019

One option would be to not emit the warning until the function is actually called:

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index 40ad964f68..7f0f3be314 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -762,8 +762,6 @@ function setupGlobalEncoding() {
 function setupQueueMicrotask() {
   Object.defineProperty(global, 'queueMicrotask', {
     get() {
-      process.emitWarning('queueMicrotask() is experimental.',
-                          'ExperimentalWarning');
       const { queueMicrotask } =
         NativeModule.require('internal/queue_microtask');
 
diff --git a/lib/internal/queue_microtask.js b/lib/internal/queue_microtask.js
index 6ca0c1e144..f8afda6d76 100644
--- a/lib/internal/queue_microtask.js
+++ b/lib/internal/queue_microtask.js
@@ -7,8 +7,15 @@ const {
   enqueueMicrotask,
   triggerFatalException
 } = internalBinding('util');
+let emitExperimentalWarning = true;
 
 function queueMicrotask(callback) {
+  if (emitExperimentalWarning) {
+    emitExperimentalWarning = false;
+    process.emitWarning('queueMicrotask() is experimental.',
+                        'ExperimentalWarning');
+  }
+
   if (typeof callback !== 'function') {
     throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
   }

@zloirock
Copy link
Author

As another option, since it's a getter, I can check that queueMicrotask property of the global object is an accessor and ignore native implementation. However, it would be better to use native implementation if it's possible.

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

Well it will eventually be removed from experimental status and then you can ship your library with it.

In the specific case of queueMicrotask, we're waiting for other browsers to ship it. I guess since that has happened now, and since the feature is stable, we could remove the warning.

In the mean time, since your library is (from what I can tell) a polyfill library, why not just polyfill the feature?

@zloirock
Copy link
Author

zloirock commented Jan 20, 2019

Well it will eventually be removed from experimental status and then you can ship your library with it.

Sounds strange allow to polyfill something only when it's available stable in engines. It should work anywhere - even in IE6, even in Node 0.8, even in Node 11, but even when experimental status will be removed, it will cause a problem in Node 11.

why not just polyfill the feature?

See the comment above. Since now it's a getter, core-js can replace queueMicrotask if it's an accessor property. However, it would be better to use native implementation if it's possible and I can't be sure that Node does not leave it as an accessor in the future(ok, I see your PR).

@devsnek
Copy link
Member

devsnek commented Jan 20, 2019

@zloirock since it seems you're not actually using queueMicrotask, you can check for !global.hasOwnProperty('queueMicrotask') instead of !global.queueMicrotask to avoid triggering the getter. (you should probably do that for everything anyway)

@zloirock
Copy link
Author

zloirock commented Jan 20, 2019

@devsnek it's not so simple. It's also should be re-exported for a version without global namespace pollution and, theoretically, can be used internally in places where required microtask (for example, if someone wanna replace Node Promise implementation or in some Observable-related features).

Object.getOwnPropertyDescriptor(global, 'queueMicrotask').get is enouth for feature detection, but is it possible somehow to get an access to internal/queue_microtask for use native Node implementation?

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.
Projects
None yet
Development

No branches or pull requests

3 participants