performance.mark doesn't exist in node@16 when using jsdom globals

[msluther]

• FakeTimers version : @sinonjs/[email protected]
• Environment : node@16
• Example URL : Minimal repro: https://github.com/msluther/sinon-timers-jsdom-bug
• Other libraries you are using: global-jsdom

What did you expect to happen?

performance.mark is a function

What actually happens

performance.mark is undefined

How to reproduce

Minimal repro is available here: https://github.com/msluther/sinon-timers-jsdom-bug

Effectly though it's just this in node@16:

require('global-jsdom/register'); // This setups up jsdom and copies properties from `window` up to global, including a minimal Performance type
const sinon = require('sinon');
global.performance.mark(); // works
sinon.useFakeTimers();
global.performance.mark(); // throws as `mark` is now `undefined`

This is related to this issue: #394

From my comment on that issue:

Hrm. So, the fix works but makes the assumption that the functions on global.Performance.prototype are a superset of global.performance.* functions. This is currently not the case when using node@16 and jsdom. jsdom is injecting a global.Performance object based on the minimal W3C spec for hr-time.

This isn't necessarily a bug with either library, but feels like a gap between the two.

However, @sinon/fake-timers ends up replacing a good global.performance.mark with undefined in this situation. Should the code here use both prototypes to make sure this is the case or maybe prefer the global.performance over global.Performance?

The work around that I posted above still works in this case as long as its injected prior to sinon & jsdom, so I'm not blocked by any means, but just requires making sure that happens in my tests everywhere.

[benjamingr]

While it's not strictly a bug I think it's very reasonable to support this use case and the fix shouldn't be too hard.

[zbyte64]

This is currently impacting me, but on node v18. What would an acceptable unit test be for this?

[zbyte64]

From my own manual testing, swapping the order here:

fake-timers/src/fake-timers-src.js

Lines 1755 to 1760 in b568150

	if (hasPerformancePrototype) {
	return _global.Performance.prototype;
	}
	if (hasPerformanceConstructorPrototype) {
	return _global.performance.constructor.prototype;
	}

is enough to resolve the issue for me anyways.

[benjamingr]

@zbyte64 feel free to open a PR

[fatso83]

PR merged 🥳 Thanks a bunch