Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to preclude the need for a continuous raf loop #1328

Merged
merged 3 commits into from
Jun 5, 2024

Conversation

eoghanmurray
Copy link
Contributor

It seems to me that the raf loop is unnecessary, so I've removed it to get your feedback.

Instead we queue up a single raf callback which will clear anything added in this frame.

I've no understanding as to why this exclusivity is per animation frame, and not (say) per emitted mutation event, any insight into that?

@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2023

🦋 Changeset detected

Latest commit: 23cd976

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

@eoghanmurray
Copy link
Contributor Author

eoghanmurray commented Oct 17, 2023

Oh yeah; these changes haven't been tested!

I've been running this change in production since early March

@eoghanmurray eoghanmurray marked this pull request as ready for review April 5, 2024 21:17
@eoghanmurray
Copy link
Contributor Author

Although I have it in production, so it's not obviously breaking things, there could be some subtleties with the shadowbuffer that might need expert analysis or a test...

I wrote this based on the logic of how the weak map was cleared etc.

@JonasBa
Copy link
Contributor

JonasBa commented Apr 29, 2024

@eoghanmurray very welcome change! This was creating a lot of noise when profiling the code 👏🏼

@eoghanmurray
Copy link
Contributor Author

@JonasBa this is a hard one for anyone to review and we don't have tests on it.
Are you running this in production by any chance?

@Juice10 Juice10 merged commit d38893f into rrweb-io:master Jun 5, 2024
6 checks passed
jeffdnguyen pushed a commit to pendo-io/rrweb that referenced this pull request Jul 29, 2024
* Refactor to preclude the need for a continuous raf loop

* Apply formatting changes

* Create shadow-dom-unbusify.md

---------

Co-authored-by: eoghanmurray <[email protected]>
jeffdnguyen pushed a commit to pendo-io/rrweb that referenced this pull request Jul 29, 2024
* Refactor to preclude the need for a continuous raf loop

* Apply formatting changes

* Create shadow-dom-unbusify.md

---------

Co-authored-by: eoghanmurray <[email protected]>
billyvg pushed a commit to getsentry/rrweb that referenced this pull request Sep 17, 2024
* Refactor to preclude the need for a continuous raf loop

* Apply formatting changes

* Create shadow-dom-unbusify.md

---------

Co-authored-by: eoghanmurray <[email protected]>
jxiwang pushed a commit to amplitude/rrweb that referenced this pull request Oct 17, 2024
* Refactor to preclude the need for a continuous raf loop

* Apply formatting changes

* Create shadow-dom-unbusify.md

---------

Co-authored-by: eoghanmurray <[email protected]>
jxiwang pushed a commit to amplitude/rrweb that referenced this pull request Oct 17, 2024
* Refactor to preclude the need for a continuous raf loop

* Apply formatting changes

* Create shadow-dom-unbusify.md

---------

Co-authored-by: eoghanmurray <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants