queueMicrotask() ExperimentalWarning in Node

[shdq]

Hello, guys.

I did a quick fix for #232 not faking this method by default like nextTick, but have a couple of questions.

--trace-warnings point on _global.queueMicrotask usage in the code.

There are two places in the code:

1. Feature detection (I fixed in @ehmicky way). Not sure about that, because it gives true when property value is null or undefined. What do you think?
2. And when it stores in lolex.timers. What is the purpose of storing all the timers methods that supported by particular environment there instead of storing only just list of them? As I got from the code hijackMethod assign _queueMicrotask to _global.queueMicrotask, but not to timers.queueMicrotask. I might be incorrect.

Now it is giving warnings only with lolex.install({ toFake: ["queueMicrotask"] }).

I'm going to add some tests for this.

@benjamingr I fixed queueMicrotask() test "runs with timers and before them", which was failed, I think it was the typo.

Any comments?

[benjamingr]

Why is this set to true?

[shdq]

It gives another warning for just var lolex = require("lolex");.
Assigned native method not used later. So it's just for "yes, this feature supported". And actually we can remove it at all, because we filtering this method later inside install function to prevent faking by default. Correct me, please, if I'm wrong.

[benjamingr]

I think this is here for restoring the original queueMicrotask later

[shdq]

I thought so, but during uninstall function it restores original method from _queueMicrotask which was assigned inside hijackMethod from target[method] (_global[queueMicrotask in our case). lolex.timers not involved in this at all.

Example code on node 11.9.0:

const lolex = require("lolex");
var clock = lolex.install({ toFake: ["setTimeout", "queueMicrotask"] });

function microtask() {
  console.log("microtask");
}
function task() {
  console.log("task");
}

setTimeout(task, 0);
queueMicrotask(microtask);

clock.tick();

clock.uninstall();

setTimeout(task, 0);
queueMicrotask(microtask);

Outputs

microtask // faked method
task // faked method
(node:27136) ExperimentalWarning: queueMicrotask() is experimental.
    at get (internal/bootstrap/node.js:771:15)
    at hijackMethod (/Users/shdq/Projects/lolex-warning/node_modules/lolex/src/lolex-src.js:486:33)
    at Object.install (/Users/shdq/Projects/lolex-warning/node_modules/lolex/src/lolex-src.js:995:9)
    at Object.<anonymous> (/Users/shdq/Projects/lolex-warning/index.js:3:19)
    at Module._compile (internal/modules/cjs/loader.js:734:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:745:10)
    at Module.load (internal/modules/cjs/loader.js:626:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:566:12)
    at Function.Module._load (internal/modules/cjs/loader.js:558:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:797:12)
microtask // native method
task // native method

It gives this warning on line 2, because there in hijackMethod _queueMicrotask = _global[queueMicrotask];

I'm not insisting on true, we can even remove it at all, because this method is not going to be faked by default after this PR, but maybe it will be against whole concept of storing all the supported timers in lolex.timers. What do you think?

[benjamingr]

1. What do you think?

I think the property check fix is fine since there is no property called enqueueMicrotask on the process object otherwise

[ehmicky]

@shdq thanks for taking some time fixing my issue!

About hasOwnProperty() you are right it does not work when the property is explicitely set to undefined. I am not sure whether some libraries or environments might do this. As @benjamingr is pointing out, a default Node.js old environment would not do this.

Either way: would chaining global.hasOwnProperty("queueMicrotask") && typeof global.queueMicrotask === "function" fix this? The right side would only get evaluated if the left side is true.

[shdq]

Either way: would chaining global.hasOwnProperty("queueMicrotask") && typeof global.queueMicrotask === "function" fix this? The right side would only get evaluated if the left side is true.

Thank you for the suggestion, when the right side will be evaluated it gives a warning in a console.

[ehmicky]

@shdq Yes you're right my suggestion did not make much sense :)

[boneskull]

Would love to see this merged; anything I can do to help?

[benjamingr]

@boneskull sorry, had a lot on my plate lately. @fatso83 can you do a release?

(If not I'll get one next week + hopefully the setTimeout promises thing)

[ehmicky]

I tried it and it works. Thanks!

[fatso83]

@benjamingr I tried doing it in a hurry on a mac I was able to loan, but npm version patch failed me:

scripts/preversion.sh 
scripts/preversion.sh: line 8: conditional binary operator expected

So it will have to wait until next week for me. Perhaps @mroderick ?

[fatso83]

OK, I am having a stab at this now.

[fatso83]

Out as lolex@4