Interpretter clock.setTimeout method does not work in Firefox WebExtension content scripts by default

[blimmer]

Description

When an XState machine with delayed events/transitions is used in a Firefox WebExtension content script, the default implementation of clock.setTimeout and clock.clearTimeout throws this exception:

TypeError: 'setTimeout' called on an object that does not implement interface Window.

Expected Result

I expected the machine, as defined, to work without throwing an exception.

Actual Result

An exception is thrown, and the state machine does not work properly.

Reproduction

Because this happens in a Firefox WebExtension, I couldn't use any of the recommended reproduction templates. Instead, I created a sample repository to show the issue: https://github.com/blimmer/xstate-webext-setTimeout-issue. There is information in the Quick Start section of the README that explains how to run the example.

Additional context

This issue seems to be caused by the default implementation of setTimeout/clearTimeout defined here: https://github.com/davidkpiano/xstate/blob/844c2c30d6160763ab164dc0592e33a942746361/packages/core/src/interpreter.ts#L125-L132

I believe the issue is with thisArg being null. I locally changed the default implementation to look like this (note the global thisArg) and the state machine started working with the default implementation.

clock: {
  setTimeout: (fn, ms) => {
    return global.setTimeout.call(global, fn, ms);
  },
  clearTimeout: (id) => {
    return global.clearTimeout.call(global, id);
  }
},

I was looking at the history and noticed that @davidkpiano change the default clock behavior in this commit to use the .call(null, ...args) pattern. This makes sense since you want setTimeout bound to the global instance passed in, but I think Firefox is doing some additional checks on the thisArg which causes this error.

[davidkpiano]

@blimmer Thanks for the investigation! I'll fix this soon.

[blimmer]

No problem at all! Thank you so much for the quick response, I really appreciate it.

Does the global thisArg make sense as a fix? If so, I'd be happy to open a PR.

[davidkpiano]

No problem at all! Thank you so much for the quick response, I really appreciate it.

Does the global thisArg make sense as a fix? If so, I'd be happy to open a PR.

Yes it does; I'd appreciate a PR!

For tests, this might be hard to test since it's a Firefox-specific thing, so as long as the existing tests pass, that's sufficient.

[Andarist]

It probably should be enough to just remove global. from those calls. I cant name any JS env in which those (setTimeout & clearTimeout) wouldnt be globally available.

[davidkpiano]

It probably should be enough to just remove global. from those calls. I cant name any JS env in which those (setTimeout & clearTimeout) wouldnt be globally available.

I can't remember why, but there was a reason I used global, whereas setTimeout/clearTimeout wouldn't work.

[blimmer]

Yeah, I was wondering what the background was on why the global call was originally added. The commit message made it seem like there was something you ran into that required it.

66c8cf2

Cleanup, fixing global setTimeout/clearTimeout issues

[blimmer]

For those linked here from the changelog, we decided to call the global methods directly as @Andarist recommended. More details available in the PR comments: #1704