use native setTimeout when zone.js is present

[mdellanoce]

This is similar to what is already done with zone.js and MutationObserver. I ran into an issue on a customer's application where the setTimeout in hookSetter was triggering angular's change detection, which caused a mutation, which led to another setTimeout, and just continued on like that forever, effectively locking up the browser at 100% CPU.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: cceada935c9aea088babda6262e0b515004e889d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
rrweb	Patch
rrweb-snapshot	Patch
rrdom	Patch
rrdom-nodejs	Patch
rrweb-player	Patch
@rrweb/types	Patch
@rrweb/web-extension	Patch
rrvideo	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mdellanoce]

Just had this happen on another customer's site, so seems somewhat common for Angular apps.

[mdellanoce]

zone.js strikes again... added a similar commit for addEventListener. This one wasn't a performance issue, but the passive mousemove listener added by rrweb forces all mousemove listeners added through zone.js to be passive as well.

[Juice10]

Looks like you've replace all of the setTimeout in rrweb's record code to nativeSetTimeout, but rrweb's record also invokes rrweb-snapshot which also includes a number of setTimeout's

[Juice10]

Thanks for submitting this fix. This PR doesn't prevent people from accidentally using the hijacked setTimeout whenever they add something to the code. It would be great if we could make sure this doesn't happen accidentally.

Some potential solutions I came up with but feel free to solve this in another way if you'd like:

• We could handle replacing setTimeout with our nativeSetTimeout in the rrweb bundling step (you could use https://www.npmjs.com/package/@rollup/plugin-replace if you'd like).
• We could add eslint rule to disallow people from calling setTimeout in all recording code.

[mdellanoce]

@Juice10 I updated rrweb-snapshot's setTimeout calls. Those seem unlikely to lead to endless looping like what I've seen with hookSetter, but I guess it doesn't hurt.

This PR doesn't prevent people from accidentally using the hijacked...

I'm struggling with this. In the past I've used the inject plugin to do this sort of thing, but it fails to parse the files if I add it before the typescript plugin, and it just plain doesn't work if I add it after the typescript plugin. Replace could still maybe work, but feels fragile.

Looked into custom eslint rules, but the eslint config seems to apply to record and replay, and I'm not sure how to add a rule that only targets recording. Maybe its okay to use for replay too?

At any rate, I'm running out of cycles to devote to this issue, and need to move on to some of our other problems with recording.

[mdellanoce]

a test failed, but i'm not sure why...

[mdellanoce]

added a similar fix for requestAnimationFrame in ProcessedNodeManager. If #1328 ever merges that commit can be dropped