NodeJS "timers" module is not mocked

[swenzel-arc]

• FakeTimers version : v10.1.0
• Environment : Node.js v18.12.1
• Other libraries you are using: jest v29.5.0

What did you expect to happen?
NodeJS's timers module to be mocked upon FakeTimers.install().

What actually happens
It's not mocked.

How to reproduce

let timers = require("timers");
let FakeTimers = require("@sinonjs/fake-timers");

let clock;

beforeEach(() => {
    clock = FakeTimers.install();
});

it("doesn't work", () => {
    let timerDone = false;
    timers.setTimeout(()=>{timerDone=true;}, 1000);
    clock.tick(1001);
    expect(timerDone).toBe(true);
});

afterEach(() => {
    clock.uninstall();
});

I was testing some code that is working with socket.io and was wondering why my sockets kept closing with ping time outs. Turns out engine.io is using timers.* instead of the corresponding global functions.
TBH I'm not quite sure if a fix belongs into this library or into jest's useFakeTimers function. However, the README here says it'd mock all "native" timers when FakeTimers.install is called without arguments, so it should maybe also include NodeJS's core module timers.

[benjamingr]

PR welcome :)

[fatso83]

@benjamingr You are bit more into the Node internals than me, so I just vaguely remember this or something related (HTTP Headers in http using primordials?) coming up earlier. Are these timers (or the internal) easily stubbable somehow? Not totally sure where a volunteer would start. Jest would probably have an easier time doing this by just stubbing out calls to the timers lib, but we would somehow need to interface with the library directly. I would assume that trying to modify the exports directly would not work (this being ESM, using a immutable namespace and all).

[benjamingr]

The timers should be pretty easily stubbable unlike HTTP headers - just require('timers') and replace that object's setTimeout/setInterval properties

[swenzel-arc]

Thanks for the quick responses 🙏

PR welcome :)

I've had a quick glance at the fake-timers repo before opening this issue... but I wouldn't know where to put the code or how to make sure it's only done for NodeJS. Also I'm a senior Python developer, not so much JS/TS 😅

Jest would probably have an easier time doing this by just stubbing out calls to the timers lib

That's what I thought too and why I'm not sure whether a fix would belong here or there...

The timers should be pretty easily stubbable unlike HTTP headers - just require('timers') and replace that object's setTimeout/setInterval properties

Indeed they are and that's exactly my current workaround:

testUtils.ts

export function myUseFakeTimers() {
  jest.useFakeTimers();
  timers.setTimeout = setTimeout;
  timers.clearTimeout = clearTimeout;
  timers.setInterval = setInterval;
  timers.clearInterval = clearInterval;
  timers.setImmediate = setImmediate;
  timers.clearImmediate = clearImmediate;
}

export function myUseRealTimers() {
  jest.useRealTimers();
  timers.setTimeout = setTimeout;
  timers.clearTimeout = clearTimeout;
  timers.setInterval = setInterval;
  timers.clearInterval = clearInterval;
  timers.setImmediate = setImmediate;
  timers.clearImmediate = clearImmediate;
}

[fatso83]

@swenzel-arc You already have code that works, so that is great. Now what about the interface ... We need to be able to selectively disable or enable the stubbing. I guess it makes sense to enable it as the default? And how should we deal with only stubbing selected timers? I think a reasonable assumption is that if you only want to stub out two specific timers from global, you would not want another selection for Node's timers module. If that assumption does not hold, we need a way of specifying it.

fakeTimers.install({ toFake: ['setTimeout', 'clearTimeout', 'nodeTimers']});

You would then basically add a section here to specifically deal with the Node timers lib.

I wouldn't know where to put the code or how to make sure it's only done for NodeJS
This is the real question. If using CommonJS one can simply do checks to see if we are running Node or if the library can be loaded successfully, and only try stubbing if that is successful. Something like

let nodeTimersModule;
try { nodeTimersModule = require('timers') }
catch(err) { // not recent Node version  }

....
if (toFake.nodeTimers && nodeTimersModule) {
   installNodeTimers()
}

Conditional requires is not possible (AFAIK) when using ESM, as linking takes place before running the code, so I guess this would prevent us from easily going the ESM in the future? If we did, I think it would require clients to use something that captured calls to timers, which seems like a big nuisance. Unless you have some clever tricks to avoid this issue, @benjamingr?

---

Word of warning: instead of having an explicit section for dealing with Node timers, one could think (as I did) that we could just use the library's ability to target a specific object with its withGlobal(target) export. The whole installNodeTimers thing would then almost just be down to a single line: installNodeTimers(){ nodeTimersClock = withGlobal(nodeTimersModule); }. We could then just restore that as nodeTimersClock.restore() when restoring everything else. Unfortunately, you would then have to sync two sets of internal clocks (for instance when doing clock.tick()), which I do not think is complexity we want :-)

[benjamingr]

Conditional requires is not possible (AFAIK) when using ESM, as linking takes place before running the code, so I guess this would prevent us from easily going the ESM in the future? If we did, I think it would require clients to use something that captured calls to timers, which seems like a big nuisance. Unless you have some clever tricks to avoid this issue, @benjamingr?

A loader (not fun) or using top level await with dynamic import (easy and also works)

[fatso83]

AFAIK, we cannot start using promise based code without changing the API. If we were to do dynamic imports (great idea, btw), we would probably have to change all the exports to resolve to promises:

export install(config:Config|null): Promise<Clock> 
export withGlobal(config:Config|null): Promise<Clock>
etc

Otherwise we would not know if the call to timers() was resolved or not.

let asyncFunction = async () => {
  async function main() {
    var value = await Promise.resolve("Hey there");
    console.log("inside: " + value);
    return value;
  }

  var text = await main();
  console.log("outside: " + text);
};

console.log("pre");
asyncFunction();
console.log("post");

will print

pre
post
inside: Hey there
outside: Hey there

Either that, or expose a promise one could wait for: await clock.nodeTimers (or clock.asyncImportsResolved or similar).

[swenzel-arc]

It appears to be a bit harder than I thought... especially the uninstall part. Looks like clocks are supposed to hijack a single object and if there's more than one, then the mechanism for keeping track of hijacked methods has to be changed.
I guess I can do that, but seems to be a pretty invasive change.

[swenzel-arc]

Or actually, nevermind... I think I'll just handle the "timers" module separately. Similar to how I solved it in the workaround.

[fatso83]

That's fine. We can still keep this issue around as we might want this still

[benjamingr]

Should this be closed or kept open for timers/promises?

[fatso83]

Ah, good catch. I'll create a new one, referencing this.

[swenzel-arc]

Thanks guys 🙏

As it turns out, this doesn't solve my initial problem, though. Jest is using a custom global object but I made it so that the timers module isn't touched unless the patched object is the "default" global object. So upon jest.useFakeTimers the timers module stays the same.
Despite that, I still think it's not a good idea to have fake-timers change the timers module for every call to "install" as this might lead to unexpected behavior if multiple objects are being patched and reverted.

[fatso83]

the referenced issue is # #469 btw

[benjamingr]

@SimenB is there anything we can do better in this regard for jest?

[SimenB]

Sniffing out require should work fine in Jest as that should work the same as Node, but that won't work for import declarations or expressions. I think that would need to hook into the module mocking directly. I haven't looked into it, tho

[swenzel-arc]

In case anyone want's to check it out:
Jest creates the custom global object here which is then passed to their wrapper a few lines down here.
The wrapper implementation is here.

Is there another way to access the "currently active global object"? Maybe we could compare the _global in withGlobal to something other than globalObject... which makes me wonder, with jest swapping out the global object, would the default install even patch the globals correctly?