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

* 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
rrweb-io · Aug 3, 2023 · 7103625 · 7103625
1 parent d872d28
commit 7103625
Show file tree

Hide file tree

Showing 5 changed files with 305 additions and 42 deletions.
diff --git a/.changeset/attribute-text-reductions.md b/.changeset/attribute-text-reductions.md
@@ -0,0 +1,5 @@
+---
+'rrweb': patch
+---
+
+Don't include redundant data from text/attribute mutations on just-added nodes
diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts
@@ -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,

diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap
@@ -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\\": [
         {

diff --git a/packages/rrweb/test/__snapshots__/record.test.ts.snap b/packages/rrweb/test/__snapshots__/record.test.ts.snap
@@ -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`] = `
 "[
   {