getsentry / rrweb

record and replay the web
https://www.rrweb.io/
Other
9 stars 4 forks source link

feat: Ensure to use unwrapped versions of `setTimeout` / `clearTimeout` #176

Closed mydea closed 2 months ago

mydea commented 2 months ago

Let's see if that helps with Angular performance some more...!

Closes https://github.com/getsentry/sentry-javascript/issues/11661 (hopefully...)

github-actions[bot] commented 2 months ago

size-limit report 📦

Path Size
rrweb - record only (gzipped) 16.8 KB (+0.2% 🔺)
rrweb - record & CanvasManager only (gzipped) 19.61 KB (+0.14% 🔺)
rrweb - record only (min) 57.24 KB (+0.07% 🔺)
rrweb - record with treeshaking flags (gzipped) 15.59 KB (+0.22% 🔺)
mydea commented 2 months ago

Very interesting! It kinda makes sense after thinking about it. ZoneJS patches all of these global APIs so let's see 🍿

(long term this might phase out with Angular becoming zone-less in favor of signals but this will take a long time until it's used broadly)

Yeah, I'd say the bundle size hit is acceptable here, and we can revert it later if we want/need to!

mydea commented 2 months ago

🙏

We will also want to do this for the setTimeouts in our replay package

Let's ship this, I'd say, and see if it already improves things, than we can also add this to replay itself, at least in hot paths related to rrweb events etc!

billyvg commented 2 months ago

🙏 We will also want to do this for the setTimeouts in our replay package

Let's ship this, I'd say, and see if it already improves things, than we can also add this to replay itself, at least in hot paths related to rrweb events etc!

@mydea when I was debugging, I saw that it was being triggered by our click handler in the replay package

mydea commented 2 months ago

🙏 We will also want to do this for the setTimeouts in our replay package

Let's ship this, I'd say, and see if it already improves things, than we can also add this to replay itself, at least in hot paths related to rrweb events etc!

@mydea when I was debugging, I saw that it was being triggered by our click handler in the replay package

sad! I guess let's implement it there as well then, will add some bundle size but I guess it's worth it!