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

snapshot is produced with non-unique IDs #76

Open
hvpavel opened this issue May 12, 2021 · 2 comments
Open

snapshot is produced with non-unique IDs #76

hvpavel opened this issue May 12, 2021 · 2 comments

Comments

@hvpavel
Copy link

hvpavel commented May 12, 2021

In some cases calling snapshot(document) produces a tree with elements which IDs are not unique. In most cases these elements are the root element document and the first element from <head> e.g. <link> or <style>.

It leads to the situation where in Player the root element document with id = 1 is overwritten by some child node, that also has id = 1. In this case IncrementalSource.Event, which is fired for document, won't be correctly processed by the Player instance.

Here the assignment of non-unique IDs happen:
https://github.com/rrweb-io/rrweb-snapshot/blob/master/src/snapshot.ts#L693

@Yuyz0112
Copy link
Member

Is this happening when you snapshot a page more than once?

@hvpavel
Copy link
Author

hvpavel commented Jun 17, 2021

No, it isn't happening when requesting a page snapshot more than once.

I was able to identify that for some websites the assignment node.__sn = {/* serialized node */} gets transferred through the page reload. Probably it's happening because of particular cache configuration, but I didn't find the exact reason.

So if you create a snapshot for a page and then refresh it, you'll be able to see that, for instance, document has __sn property assigned even if you don't load rrweb-snapshot after the page refresh.

Unfortunately, I don't have any strong case to share. However, I've noticed that this issue is happening more often in Safari than in any other browser (probably only in Safari).

I was able to fix it by collecting the serialized objects into a WeakMap instead of directly modifying the Node element.

In a separate file:

export const serializedCache = new WeakMap<Node, serializedNodeWithId>();

Restoring cache:

let serializedNode = serializedCache.get(node);
// Try to reuse the previous id
if (serializedNode) {
  id = serializedNode.id;
}

Caching a serialized node:

const serializedNode = Object.assign(_serializedNode, { id });
serializedCache.set(n, serializedNode);

I can create a PR if you approve this modification.

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

No branches or pull requests

2 participants