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

fix duplicated shadow dom #1095

Merged
merged 7 commits into from
Feb 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/loud-seals-raise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'rrweb-snapshot': patch
'rrweb': patch
---

Fix duplicated shadow doms
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"lerna": "^4.0.0",
"markdownlint": "^0.25.1",
"markdownlint-cli": "^0.31.1",
"prettier": "2.8.4",
"turbo": "^1.2.4",
"typescript": "^4.7.3"
},
Expand Down
15 changes: 14 additions & 1 deletion packages/rrweb-snapshot/src/rebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
attributes,
legacyAttributes,
} from './types';
import { isElement, Mirror } from './utils';
import { isElement, Mirror, isNodeMetaEqual } from './utils';

const tagMap: tagMap = {
script: 'noscript',
Expand Down Expand Up @@ -364,6 +364,19 @@ export function buildNodeWithSN(
afterAppend,
cache,
} = options;
/**
* Add a check to see if the node is already in the mirror. If it is, we can skip the whole process.
* This situation (duplicated nodes) can happen when recorder has some unfixed bugs and the same node is recorded twice. Or something goes wrong when saving or transferring event data.
* Duplicated node creation may cause unexpected errors in replayer. This check tries best effort to prevent the errors.
*/
if (mirror.has(n.id)) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const nodeInMirror = mirror.getNode(n.id)!;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const meta = mirror.getMeta(nodeInMirror)!;
// For safety concern, check if the node in mirror is the same as the node we are trying to build
if (isNodeMetaEqual(meta, n)) return mirror.getNode(n.id);
}
let node = buildNode(n, { doc, hackCss, cache });
if (!node) {
return null;
Expand Down
33 changes: 33 additions & 0 deletions packages/rrweb-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import {
nodeMetaMap,
IMirror,
serializedNodeWithId,
serializedNode,
NodeType,
documentNode,
documentTypeNode,
textNode,
elementNode,
} from './types';

export function isElement(n: Node): n is Element {
Expand Down Expand Up @@ -213,3 +219,30 @@ export function is2DCanvasBlank(canvas: HTMLCanvasElement): boolean {
}
return true;
}

export function isNodeMetaEqual(a: serializedNode, b: serializedNode): boolean {
if (!a || !b || a.type !== b.type) return false;
if (a.type === NodeType.Document)
return a.compatMode === (b as documentNode).compatMode;
else if (a.type === NodeType.DocumentType)
return (
a.name === (b as documentTypeNode).name &&
a.publicId === (b as documentTypeNode).publicId &&
a.systemId === (b as documentTypeNode).systemId
);
else if (
a.type === NodeType.Comment ||
a.type === NodeType.Text ||
a.type === NodeType.CDATA
)
return a.textContent === (b as textNode).textContent;
else if (a.type === NodeType.Element)
return (
a.tagName === (b as elementNode).tagName &&
JSON.stringify(a.attributes) ===
JSON.stringify((b as elementNode).attributes) &&
Comment on lines +242 to +243
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite a slow call to do, aren't you afraid this will slow down repeated rrweb.takeFullSnapshot(true)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also what happens if a node gets a new attribute since the last time it was snapshotted? Will this node get added twice?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite a slow call to do, aren't you afraid this will slow down repeated rrweb.takeFullSnapshot(true)?

Yes you are right, this function is slow. It's in the rebuilt.ts so I don't think this can slow down the takeFullSnapshot function. If the data events are correct, the program won't go to this path. This comparison will only be used when rrweb ingests any problematic data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also what happens if a node gets a new attribute since the last time it was snapshotted? Will this node get added twice?

If the recorder works well, the incremental data will only change the node's attribute rather than totally recreate the node through this path.
In case the program goes here, the node will get added twice as you said. But honestly, I don't know what kind of error event data can lead to this. I wrote this function as a fallback.
Do you have any suggestions for improving the code here?

a.isSVG === (b as elementNode).isSVG &&
a.needBlock === (b as elementNode).needBlock
);
return false;
}
150 changes: 150 additions & 0 deletions packages/rrweb-snapshot/test/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/**
* @jest-environment jsdom
*/
import { NodeType, serializedNode } from '../src/types';
import { isNodeMetaEqual } from '../src/utils';
import { serializedNodeWithId } from 'rrweb-snapshot';

describe('utils', () => {
describe('isNodeMetaEqual()', () => {
const document1: serializedNode = {
type: NodeType.Document,
compatMode: 'CSS1Compat',
childNodes: [],
};
const document2: serializedNode = {
type: NodeType.Document,
compatMode: 'BackCompat',
childNodes: [],
};
const documentType1: serializedNode = {
type: NodeType.DocumentType,
name: 'html',
publicId: '',
systemId: '',
};
const documentType2: serializedNode = {
type: NodeType.DocumentType,
name: 'html',
publicId: '',
systemId: 'http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd',
};
const text1: serializedNode = {
type: NodeType.Text,
textContent: 'Hello World',
};
const text2: serializedNode = {
type: NodeType.Text,
textContent: 'Hello world',
};
const comment1: serializedNode = {
type: NodeType.Comment,
textContent: 'Hello World',
};
const comment2: serializedNode = {
type: NodeType.Comment,
textContent: 'Hello world',
};
const element1: serializedNode = {
type: NodeType.Element,
tagName: 'div',
attributes: {
className: 'test',
},
childNodes: [],
};
const element2: serializedNode = {
type: NodeType.Element,
tagName: 'span',
attributes: {
'aria-label': 'Hello World',
},
childNodes: [],
};
const element3: serializedNode = {
type: NodeType.Element,
tagName: 'div',
attributes: { id: 'test' },
childNodes: [comment1 as serializedNodeWithId],
};

it('should return false if two nodes have different node types', () => {
expect(
isNodeMetaEqual(
undefined as unknown as serializedNode,
null as unknown as serializedNode,
),
).toBeFalsy();
expect(isNodeMetaEqual(document1, element1)).toBeFalsy();
expect(isNodeMetaEqual(document1, documentType1)).toBeFalsy();
expect(isNodeMetaEqual(documentType1, element1)).toBeFalsy();
expect(isNodeMetaEqual(text1, comment1)).toBeFalsy();
expect(isNodeMetaEqual(text1, element1)).toBeFalsy();
expect(isNodeMetaEqual(comment1, element1)).toBeFalsy();
});

it('should compare meta data of two document nodes', () => {
expect(
isNodeMetaEqual(document1, JSON.parse(JSON.stringify(document1))),
).toBeTruthy();
expect(
isNodeMetaEqual(JSON.parse(JSON.stringify(document2)), document2),
).toBeTruthy();
expect(isNodeMetaEqual(document1, document2)).toBeFalsy();
});

it('should compare meta data of two documentType nodes', () => {
expect(
isNodeMetaEqual(
documentType1,
JSON.parse(JSON.stringify(documentType1)),
),
).toBeTruthy();
expect(
isNodeMetaEqual(
JSON.parse(JSON.stringify(documentType2)),
documentType2,
),
).toBeTruthy();
expect(isNodeMetaEqual(documentType1, documentType2)).toBeFalsy();
});

it('should compare meta data of two text nodes', () => {
expect(
isNodeMetaEqual(text1, JSON.parse(JSON.stringify(text1))),
).toBeTruthy();
expect(
isNodeMetaEqual(JSON.parse(JSON.stringify(text2)), text2),
).toBeTruthy();
expect(isNodeMetaEqual(text1, text2)).toBeFalsy();
});

it('should compare meta data of two comment nodes', () => {
expect(
isNodeMetaEqual(comment1, JSON.parse(JSON.stringify(comment1))),
).toBeTruthy();
expect(
isNodeMetaEqual(JSON.parse(JSON.stringify(comment2)), comment2),
).toBeTruthy();
expect(isNodeMetaEqual(comment1, comment2)).toBeFalsy();
});

it('should compare meta data of two HTML elements', () => {
expect(
isNodeMetaEqual(element1, JSON.parse(JSON.stringify(element1))),
).toBeTruthy();
expect(
isNodeMetaEqual(JSON.parse(JSON.stringify(element2)), element2),
).toBeTruthy();
expect(
isNodeMetaEqual(element1, {
...element1,
childNodes: [comment2 as serializedNodeWithId],
}),
).toBeTruthy();
expect(isNodeMetaEqual(element1, element2)).toBeFalsy();
expect(isNodeMetaEqual(element1, element3)).toBeFalsy();
expect(isNodeMetaEqual(element2, element3)).toBeFalsy();
});
});
});
2 changes: 0 additions & 2 deletions packages/rrweb/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"@types/jest-image-snapshot": "^5.1.0",
"@types/node": "^17.0.21",
"@types/offscreencanvas": "^2019.6.4",
"@types/prettier": "^2.3.2",
"@types/puppeteer": "^5.4.4",
"construct-style-sheets-polyfill": "^3.1.0",
"cross-env": "^5.2.0",
Expand All @@ -65,7 +64,6 @@
"jest": "^27.5.1",
"jest-image-snapshot": "^5.2.0",
"jest-snapshot": "^23.6.0",
"prettier": "2.2.1",
"puppeteer": "^11.0.0",
"rollup": "^2.68.0",
"rollup-plugin-esbuild": "^4.9.1",
Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb/src/record/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,8 @@ function record<T = eventWithTime>(

// When we take a full snapshot, old tracked StyleSheets need to be removed.
stylesheetManager.reset();
// Old shadow doms cache need to be cleared.
shadowDomManager.clearCache();

shadowDomManager.init();

mutationBuffers.forEach((buf) => buf.lock()); // don't allow any mirror modifications during snapshotting
const node = snapshot(document, {
Expand Down
Loading