`install({ toFake: ["setImmediate"] })` fails when `setImmediate` is not available in context (such as browser environment)

[hi-ogawa]

• FakeTimers version : v11.2.2
• Environment : reproduction is in stackblitz nodejs, but it's likely in any environment since reproduction uses withGlobal to pass context manually
• Example URL : https://stackblitz.com/edit/vitest-dev-vitest-yzjgau?file=repro-setImmediate.mjs
• Other libraries you are using: FakeTimers is only dependency in reproduction

What did you expect to happen?

No error. Preferably allow faking setImmediate even when it's not available in the context, which would align with current behavior with requestIdleCallback, requestAnimationFrame etc... (see repro-other.mjs in reproduction).

What actually happens

Error: Cannot set properties of undefined (setting 'hadOwnProperty')

How to reproduce

import { withGlobal } from '@sinonjs/fake-timers';

function main() {
  const global = { Date, setTimeout, clearTimeout };
  const timers = withGlobal(global);

  // error when install
  // > Error: Cannot set properties of undefined (setting 'hadOwnProperty')
  const clock = timers.install({ toFake: ['setImmediate'] });
}

main();

extra context of the issue

First of all, thanks for making this portable library for timer mocking! I found some inconsistency of @sinonjs/fake-timers while investigating an issue on Vitest https://github.com/vitest-dev/vitest/issues/5027#issuecomment-1903570906. There is a workaround on Vitest side so it's not critical, but I thought it's worth reporting an issue.

This is likely a same issue as https://github.com/sinonjs/fake-timers/issues/277 but I think the issue still persists since the verification https://github.com/sinonjs/fake-timers/issues/277#issuecomment-851504314 seems to be done on NodeJS where setImmediate is globally available but this is not the case on browsers.

I'm not sure what's desired behavior, but there seems slight inconsistency whether @sinonjs/fake-timers fakes what's not globally available. From what I tried so far, { toFake: ["requestIdleCallback"] } works even when it's not globally available (for example, this is browser-only and not available on NodeJS). On the other hand, { toFake: ["setImmediate"] } throws an error on browser.

[fatso83]

Good call. Whatever we do, it should be consistent. I usually like to be strict (as in throwing an error, possibly overridable with a flag), but we could also do the reverse.

[SimenB]

Failing with an explicit error in either case seems reasonable, but that might be considered a breaking change?

[hi-ogawa]

Thanks for the follow up.

Failing with an explicit error in either case seems reasonable, but that might be considered a breaking change?

I think stop faking/providing requestIdleCallback when it's not globally available (such as plain NodeJS) would be a breaking change since that's essentially what happened with Vitest https://github.com/vitest-dev/vitest/issues/5027#issuecomment-1903451279

I haven't checked properly yet, but it seems jsdom/happy-dom doesn't provide requestIdleCallback global either while they do provide requestAnimationFrame etc... If they were providing requestIdleCallback, then I could've suggested users to switch to jsdom/happy-dom, so In some sense, this could be considered jsdom/happy-dom side issue as well. I'd love to hear what you think. (EDIT: I mentioned jsdom here but I guess this is a different matter. I just thought worth mentioning since inconsistency visible to users at the test framework level can potentially depend on jsdom usage.)

[fatso83]

Failing with an explicit error in either case seems reasonable, but that might be considered a breaking change?

Absolutely, but I would also add a flag to silence those errors to have an easy fallback to the previous behaviour. That way Jest could consume fake-timers@12 by passing ignoreMissingTimers: true (just a suggestion) by default.

[fatso83]

Added the breaking change in #491 with a flag to ignore missing. That should make the behaviour consistent going forwards and maybe make introducing #323 simpler. Took a lot more effort to get the testcases working than expected, but at least it is now working.

Have a look, otherwise I'll just merge in a few days.