[Feature]: In Node, hijack perf_hooks.performance in addition to global.performance

[kevinyou]

• FakeTimers version : 10.0.2
• Environment : Node v18.12.1
• Example URL : https://github.com/kevinyou/wtf-performance-now-mocking/tree/main/sinon-fake-timers
• Other libraries you are using: N/A

What did you expect to happen? After calling useFakeTimers(), perf_hooks.performance.now() should return a fake time, just like how performance.now() does.

What actually happens 1. perf_hooks.performance.now() is not the same as performance.now(). The latter is correctly hijacked and returns a fake time. 2. perf_hooks.performance.now() returns a negative value -- undefined behavior?

How to reproduce https://github.com/kevinyou/wtf-performance-now-mocking/tree/main/sinon-fake-timers

var perf_hooks = require('perf_hooks');
var FakeTimers = require("@sinonjs/fake-timers");

console.log(perf_hooks.performance === performance); // true
console.log(perf_hooks.performance.now()); // 36.97510700020939
console.log(performance.now()); // 37.08340699970722

var clock = FakeTimers.install({
    now: Date.now(),
    shouldClearNativeTimers: true,
});
console.log(perf_hooks.performance === performance); // now false
console.log(perf_hooks.performance.now()); // -4217337.086744
console.log(performance.now()); // 0

[fatso83]

This is starting to get a bit exotic ... Think you could get away with simply faking the required exports yourself?

[kevinyou]

Yes, as a workaround I did that (in Jest) and it works:

jest.mock('perf_hooks', () => ({
    performance: {
        now: () => global.performance.now(),
    },
}));

But as a user, it was very confusing until I dug deep to the fake-timers source code. The documentation talks about overriding performance.now() but whether this applies to the one in perf_hooks was unclear. Also, it wasn't until Node v16 that performance became a global so older libraries still access it through perf_hooks.

Maybe the documentation could have a disclaimer about performance in perf_hooks, like in the config.toFake row notes.

[benjamingr]

I think it's probably fine to support it and not a lot of work?

I'm happy to make changes in Node if that'd help

[fatso83]

@benjamingr I am at a bit of a loss as to how to do this cleanly without hooking into module loader territory, so if you have a way of doing this cleanly by exposing something in Node, then by all means feel free ❤️

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[fatso83]

@benjamingr Know if something for us to hook into will arrive?

[dmurvihill]

+1, this will definitely help users of limiter. Particularly since Jest can no longer easily use @kevinyou's workaround with ESM.

[fatso83]

@dmurvihill If you know how we can do this in some reasonable manner, then feel free to supply a PR. I do not know how. To the best of my knowledge, this would venture into module loader territory, like what I am doing in this article.

On the other hand, node:perf_hooks is a Node API, so maybe Node exposes some hook for us to use, but I would not know.

[dmurvihill]

I ended up moving away from the affected dependency entirely, but if it comes up again I'll come back for another look.

[fatso83]

Never got a reply from Node folks on a related question, so this is requiring a deeper look from someone interested.

[fatso83]

Kevin's general approach works fine - ESM or no ESM (if you exclude Jest specifics): while the exports are not assignable, their fields are. So we can easily use the same approach in fake-timers:

 const perf_hooks = await import("perf_hooks")
perf_hooks.performance.now = () => 42

Should not be much work. I just (erroneously) assumed we needed to replace the entire module.