-
Notifications
You must be signed in to change notification settings - Fork 140
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
⚡️ [RUMF-902] enable new mutation observer #842
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🔥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense to me, thanks for the cleanup Benoit!
the follow up plan seem good as well, I'll be happy to see those PRs
Codecov Report
@@ Coverage Diff @@
## main #842 +/- ##
==========================================
+ Coverage 86.75% 88.25% +1.50%
==========================================
Files 82 81 -1
Lines 3918 3670 -248
Branches 884 821 -63
==========================================
- Hits 3399 3239 -160
+ Misses 519 431 -88
Continue to review full report at Codecov.
|
Motivation
Enable the mutation observer introduced in #810
Changes
MutationController
intomutationObserver
Note
While working on this, I started thinking about a more in-depth cleaning in two ways:
redesign the 'rrweb-snapshot' logic. I did not forget about this comment, and I think we can drastically simplify the design of the whole file, for example to avoid serializing ignored nodes.
move modules around. I'd like to:
rrweb-snapshot
(serialization logic) and theprivacy
module closer to the rest of the 'rrweb' codebasedomain/rrweb
todomain/record
domain/{segment,segmentCollection,deflateWorker}
todomain/segmentCollection/*
This will be done later, in separate PRs though.
Testing
Unit, E2E
I have gone over the contributing documentation.