Remove INode (node.__sn) and use Mirror as source of truth
#868
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
YunFeng0817
requested changes
Apr 5, 2022
Member
YunFeng0817
left a comment
YunFeng0817
left a comment
There was a problem hiding this comment.
👍A great patch that will solve many issues and inconveniences in the current branch.
There are only several typecast that have space to improve.
Co-authored-by: Yun Feng <[email protected]>
Yuyz0112
requested changes
Apr 5, 2022
Member
|
This PR is a lifesaver, |
YunFeng0817
approved these changes
Apr 6, 2022
Yuyz0112
approved these changes
Apr 6, 2022
Vadman97
pushed a commit
to highlight/rrweb
that referenced
this pull request
Jun 2, 2022
* Move ids to weakmap * Fix typo * Move from INode to storing serialized data in mirror * Update packages/rrweb-snapshot/src/rebuild.ts Co-authored-by: Yun Feng <[email protected]> * Remove unnessisary `as Node` typecastings Fixes: rrweb-io/rrweb#868 (comment) * Remove unnessisary `as unknown as ...` * Remove unnessisary `as unknown as ...` * Reset mirror when recording starts Solves: rrweb-io/rrweb#868 (comment) * API has changed for snapshot, change test to reflect that * Allow for es5 compatibility * Remove unnessisary as unknown as ... and change test to reflect the API change * Refactor mirror to remove `nodeIdMap` Fixes: rrweb-io/rrweb#868 (comment) Co-authored-by: Yun Feng <[email protected]>
This was referenced Jul 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we can't record rrweb with rrweb because both the replay and recording use the
node.__snattribute which causes id collisions.To move away from
INode's (nodes with__snattribute) this PR uses theMirrorto save all serialized data, and id numbers.This PR could potentially make it easier for multiple rrweb recordings to happen at the same time without the recording sessions needing to use the exact same ids for the same nodes.
Breaking change
The mirror was moved to rrweb-snapshot, has its own class, and the
mapattribute isn't supported anymore. We could create a.mapfallback to help with backward compatibility.