xuguopeng/rrweb
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Mutation (attribute & text) duplicate info elimination (#1269)

Browse Source 
* Add a test which demonstrates how no mutations are generated when an element is created & destroyed in the same 'cycle' (a cylce here being enforced by freezePage)

* Test demonstrating current behaviour I'm about to modify; the data-test="x" attribute is present twice in the mutation, as is the textContent value of 'y'

* Attribute or text modifications on just-added nodes are redundant as demonstrated in test case

* Some correct test changes from other tests; I've manually inspected each of these mutation removals and confirmed that the attribute values are already present in the newly added nodes elsewhere in the same mutation

* Improve reliability of test case as per Justin's advice
This commit is contained in:
Eoghan Murray
2023-08-03 17:11:43 +01:00
committed by GitHub
parent d872d2809e
commit 7103625b46
5 changed files with 305 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/attribute-text-reductions.md Normal file
View File

@@ -0,0 +1,5 @@
---
'rrweb': patch
---
Don't include redundant data from text/attribute mutations on just-added nodes

6
packages/rrweb/src/record/mutation.ts
View File

@@ -265,6 +265,7 @@ export default class MutationBuffer {
// so that the mirror for takeFullSnapshot doesn't get mutated while it's event is being processed
const adds: addedNodeMutation[] = [];
const addedIds = new Set<number>();
/**
* Sometimes child node may be pushed before its newly added
@@ -335,6 +336,7 @@ export default class MutationBuffer {
nextId,
node: sn,
});
addedIds.add(sn.id);
}
};
@@ -434,6 +436,8 @@ export default class MutationBuffer {
id: this.mirror.getId(text.node),
value: text.value,
}))
// no need to include them on added elements, as they have just been serialized with up to date attribubtes
.filter((text) => !addedIds.has(text.id))
// text mutation's id was not in the mirror map means the target node has been removed
.filter((text) => this.mirror.has(text.id)),
attributes: this.attributes
@@ -460,6 +464,8 @@ export default class MutationBuffer {
attributes: attributes,
};
})
// no need to include them on added elements, as they have just been serialized with up to date attribubtes
.filter((attribute) => !addedIds.has(attribute.id))
// attribute mutation's id was not in the mirror map means the target node has been removed
.filter((attribute) => this.mirror.has(attribute.id)),
removes: this.removes,

43
packages/rrweb/test/__snapshots__/integration.test.ts.snap
View File

@@ -566,12 +566,6 @@ exports[`record integration tests can freeze mutations 1`] = `
\\"source\\": 0,
\\"texts\\": [],
\\"attributes\\": [
{
\\"id\\": 20,
\\"attributes\\": {
\\"foo\\": \\"bar\\"
}
},
{
\\"id\\": 5,
\\"attributes\\": {
@@ -2929,38 +2923,11 @@ exports[`record integration tests can record node mutations 1`] = `
\\"class\\": \\"select2-container select2-dropdown-open select2-container-active\\"
}
},
{
\\"id\\": 36,
\\"attributes\\": {
\\"id\\": \\"select2-drop\\",
\\"style\\": \\"left: Npx; width: Npx; top: Npx; bottom: auto; display: block;\\",
\\"class\\": \\"select2-drop select2-display-none select2-with-searchbox select2-drop-active\\"
}
},
{
\\"id\\": 70,
\\"attributes\\": {
\\"style\\": \\"\\"
}
},
{
\\"id\\": 42,
\\"attributes\\": {
\\"class\\": \\"select2-input select2-focused\\",
\\"aria-activedescendant\\": \\"select2-result-label-2\\"
}
},
{
\\"id\\": 35,
\\"attributes\\": {
\\"disabled\\": \\"\\"
}
},
{
\\"id\\": 72,
\\"attributes\\": {
\\"class\\": \\"select2-results-dept-0 select2-result select2-result-selectable select2-highlighted\\"
}
}
],
\\"removes\\": [
@@ -4615,15 +4582,7 @@ exports[`record integration tests handles null attribute values 1`] = `
\\"data\\": {
\\"source\\": 0,
\\"texts\\": [],
\\"attributes\\": [
{
\\"id\\": 20,
\\"attributes\\": {
\\"aria-label\\": \\"label\\",
\\"id\\": \\"test-li\\"
}
}
],
\\"attributes\\": [],
\\"removes\\": [],
\\"adds\\": [
{

206
packages/rrweb/test/__snapshots__/record.test.ts.snap
View File

@@ -1,5 +1,91 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`record aggregates mutations 1`] = `
"[
{
\\"type\\": 4,
\\"data\\": {
\\"href\\": \\"about:blank\\",
\\"width\\": 1920,
\\"height\\": 1080
}
},
{
\\"type\\": 2,
\\"data\\": {
\\"node\\": {
\\"type\\": 0,
\\"childNodes\\": [
{
\\"type\\": 1,
\\"name\\": \\"html\\",
\\"publicId\\": \\"\\",
\\"systemId\\": \\"\\",
\\"id\\": 2
},
{
\\"type\\": 2,
\\"tagName\\": \\"html\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 2,
\\"tagName\\": \\"head\\",
\\"attributes\\": {},
\\"childNodes\\": [],
\\"id\\": 4
},
{
\\"type\\": 2,
\\"tagName\\": \\"body\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 6
},
{
\\"type\\": 2,
\\"tagName\\": \\"input\\",
\\"attributes\\": {
\\"type\\": \\"text\\",
\\"size\\": \\"40\\"
},
\\"childNodes\\": [],
\\"id\\": 7
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\\\n \\\\n \\",
\\"id\\": 8
}
],
\\"id\\": 5
}
],
\\"id\\": 3
}
],
\\"id\\": 1
},
\\"initialOffset\\": {
\\"left\\": 0,
\\"top\\": 0
}
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 2,
\\"type\\": 2,
\\"id\\": 5
}
}
]"
`;
exports[`record can add custom event 1`] = `
"[
{
@@ -3349,6 +3435,126 @@ exports[`record loading stylesheets captures stylesheets that are still loading
]"
`;
exports[`record no need for attribute mutations on adds 1`] = `
"[
{
\\"type\\": 4,
\\"data\\": {
\\"href\\": \\"about:blank\\",
\\"width\\": 1920,
\\"height\\": 1080
}
},
{
\\"type\\": 2,
\\"data\\": {
\\"node\\": {
\\"type\\": 0,
\\"childNodes\\": [
{
\\"type\\": 1,
\\"name\\": \\"html\\",
\\"publicId\\": \\"\\",
\\"systemId\\": \\"\\",
\\"id\\": 2
},
{
\\"type\\": 2,
\\"tagName\\": \\"html\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 2,
\\"tagName\\": \\"head\\",
\\"attributes\\": {},
\\"childNodes\\": [],
\\"id\\": 4
},
{
\\"type\\": 2,
\\"tagName\\": \\"body\\",
\\"attributes\\": {},
\\"childNodes\\": [
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\",
\\"id\\": 6
},
{
\\"type\\": 2,
\\"tagName\\": \\"input\\",
\\"attributes\\": {
\\"type\\": \\"text\\",
\\"size\\": \\"40\\"
},
\\"childNodes\\": [],
\\"id\\": 7
},
{
\\"type\\": 3,
\\"textContent\\": \\"\\\\n \\\\n \\\\n \\",
\\"id\\": 8
}
],
\\"id\\": 5
}
],
\\"id\\": 3
}
],
\\"id\\": 1
},
\\"initialOffset\\": {
\\"left\\": 0,
\\"top\\": 0
}
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 0,
\\"texts\\": [],
\\"attributes\\": [],
\\"removes\\": [],
\\"adds\\": [
{
\\"parentId\\": 5,
\\"nextId\\": null,
\\"node\\": {
\\"type\\": 2,
\\"tagName\\": \\"div\\",
\\"attributes\\": {
\\"id\\": \\"here\\",
\\"data-test\\": \\"x\\"
},
\\"childNodes\\": [],
\\"id\\": 9
}
},
{
\\"parentId\\": 9,
\\"nextId\\": null,
\\"node\\": {
\\"type\\": 3,
\\"textContent\\": \\"y\\",
\\"id\\": 10
}
}
]
}
},
{
\\"type\\": 3,
\\"data\\": {
\\"source\\": 2,
\\"type\\": 2,
\\"id\\": 5
}
}
]"
`;
exports[`record should record scroll position 1`] = `
"[
{

87
packages/rrweb/test/record.test.ts
View File

@@ -35,6 +35,7 @@ interface IWindow extends Window {
takeFullSnapshot: (isCheckout?: boolean | undefined) => void;
};
freezePage(): void;
addCustomEvent<T>(tag: string, payload: T): void;
};
emit: (e: eventWithTime) => undefined;
@@ -651,6 +652,92 @@ describe('record', function (this: ISuite) {
assertSnapshot(ctx.events);
});
it('aggregates mutations', async () => {
await ctx.page.evaluate(() => {
return new Promise((resolve) => {
const { record, freezePage } = (window as unknown as IWindow).rrweb;
record({
emit: (window as unknown as IWindow).emit,
});
freezePage();
setTimeout(() => {
const div = document.createElement('div');
div.setAttribute('id', 'here-and-gone');
document.body.appendChild(div);
}, 0);
setTimeout(() => {
const div = document.getElementById('here-and-gone');
if (div) {
div.setAttribute('data-test', 'x');
}
}, 10);
setTimeout(() => {
const div = document.getElementById('here-and-gone');
if (div) {
div.parentNode?.removeChild(div as HTMLElement);
}
}, 15);
setTimeout(() => {
// 'unfreeze' happens upon a user event
// however, we expect none of the above mutations to produce any effect
document.body.click();
}, 20);
setTimeout(() => {
resolve(null);
}, 25);
});
});
await waitForRAF(ctx.page); // wait till events get sent
const mutationEvents = ctx.events.filter(
(e) =>
e.type === EventType.IncrementalSnapshot &&
e.data.source === IncrementalSource.Mutation,
);
expect(mutationEvents.length).toEqual(0); // there was no aggregate effect
assertSnapshot(ctx.events);
});
it('no need for attribute mutations on adds', async () => {
await ctx.page.evaluate(() => {
const { record, freezePage } = (window as unknown as IWindow).rrweb;
record({
emit: (window as unknown as IWindow).emit,
});
freezePage();
setTimeout(() => {
const div = document.createElement('div');
div.setAttribute('id', 'here');
div.innerText = 'as-created';
div.setAttribute('data-test', 'as-created');
document.body.appendChild(div);
}, 0);
setTimeout(() => {
const div = document.getElementById('here');
if (div) {
div.setAttribute('data-test', 'x');
(div.childNodes[0] as Text).replaceData(0, 'as-created'.length, 'y');
}
}, 10);
setTimeout(() => {
// 'unfreeze' happens upon a user event
document.body.click();
}, 20);
});
await ctx.page.waitForTimeout(50); // wait till setTimeout is called
await waitForRAF(ctx.page); // wait till events get sent
const mutationEvents = ctx.events.filter(
(e) =>
e.type === EventType.IncrementalSnapshot &&
e.data.source === IncrementalSource.Mutation,
);
expect(mutationEvents.length).toEqual(1);
assertSnapshot(ctx.events);
});
describe('loading stylesheets', () => {
let server: Server;
let serverURL: string;