fix remove node observer and check on the result of getNode (#43)

* check removed node and its parent before collect

* add more more checks on the result of getNode

src/record/observer.ts

@@ -102,24 +102,33 @@ function initMutationObserver(cb: mutationCallBack): MutationObserver {
         case 'childList': {
           addedNodes.forEach(n => genAdds(n));
           removedNodes.forEach(n => {
+            const nodeId = mirror.getId(n as INode);
+            const parentId = mirror.getId(target as INode);
             if (isBlocked(n)) {
               return;
             }
             // removed node has not been serialized yet, just remove it from the Set
             if (addsSet.has(n)) {
               addsSet.delete(n);
               dropped.push(n);
-            } else if (addsSet.has(target) && !mirror.getId(n as INode)) {
+            } else if (addsSet.has(target) && nodeId === -1) {
               /**
                * If target was newly added and removed child node was
                * not serialized, it means the child node has been removed
                * before callback fired, so we can ignore it.
                * TODO: verify this
                */
+            } else if (!mirror.has(parentId)) {
+              /**
+               * If parent id was not in the mirror map any more, it
+               * means the parent node has already been removed. So
+               * the node is also removed which we do not need to track
+               * and replay.
+               */
             } else {
               removes.push({
-                parentId: mirror.getId(target as INode),
-                id: mirror.getId(n as INode),
+                parentId,
+                id: nodeId,
               });
             }
             mirror.removeNodeFromMap(n as INode);
@@ -131,14 +140,6 @@ function initMutationObserver(cb: mutationCallBack): MutationObserver {
       }
     });
 
-    removes = removes.map(remove => {
-      if (remove.parentNode) {
-        remove.parentId = mirror.getId(remove.parentNode as INode);
-        delete remove.parentNode;
-      }
-      return remove;
-    });
-
     const isDropped = (n: Node): boolean => {
       const { parentNode } = n;
       if (!parentNode) {

src/replay/index.ts

@@ -328,11 +328,12 @@ export class Replayer {
         d.removes.forEach(mutation => {
           const target = mirror.getNode(mutation.id);
           if (!target) {
-            return this.warnTargetNotFound(d, mutation.id);
+            return this.warnNodeNotFound(d, mutation.id);
+          }
+          const parent = mirror.getNode(mutation.parentId);
+          if (!parent) {
+            return this.warnNodeNotFound(d, mutation.parentId);
           }
-          const parent = (mirror.getNode(
-            mutation.parentId!,
-          ) as Node) as Element;
           // target may be removed with its parents before
           mirror.removeNodeFromMap(target);
           if (parent) {
@@ -348,7 +349,10 @@ export class Replayer {
             mirror.map,
             true,
           ) as Node;
-          const parent = (mirror.getNode(mutation.parentId) as Node) as Element;
+          const parent = mirror.getNode(mutation.parentId);
+          if (!parent) {
+            return this.warnNodeNotFound(d, mutation.parentId);
+          }
           let previous: Node | null = null;
           let next: Node | null = null;
           if (mutation.previousId) {
@@ -387,18 +391,27 @@ export class Replayer {
         }
 
         d.texts.forEach(mutation => {
-          const target = (mirror.getNode(mutation.id) as Node) as Text;
+          const target = mirror.getNode(mutation.id);
+          if (!target) {
+            return this.warnNodeNotFound(d, mutation.id);
+          }
           target.textContent = mutation.value;
         });
         d.attributes.forEach(mutation => {
-          const target = (mirror.getNode(mutation.id) as Node) as Element;
+          const target = mirror.getNode(mutation.id);
+          if (!target) {
+            return this.warnNodeNotFound(d, mutation.id);
+          }
           for (const attributeName in mutation.attributes) {
             if (typeof attributeName === 'string') {
               const value = mutation.attributes[attributeName];
               if (value) {
-                target.setAttribute(attributeName, value);
+                ((target as Node) as Element).setAttribute(
+                  attributeName,
+                  value,
+                );
               } else {
-                target.removeAttribute(attributeName);
+                ((target as Node) as Element).removeAttribute(attributeName);
               }
             }
           }
@@ -415,7 +428,7 @@ export class Replayer {
                 this.mouse.style.top = `${p.y}px`;
                 const target = mirror.getNode(p.id);
                 if (!target) {
-                  return this.warnTargetNotFound(d, p.id);
+                  return this.warnNodeNotFound(d, p.id);
                 }
                 this.hoverElements((target as Node) as Element);
               },
@@ -435,7 +448,7 @@ export class Replayer {
         const event = new Event(MouseInteractions[d.type].toLowerCase());
         const target = mirror.getNode(d.id);
         if (!target) {
-          return this.warnTargetNotFound(d, d.id);
+          return this.warnNodeNotFound(d, d.id);
         }
         switch (d.type) {
           case MouseInteractions.Blur:
@@ -475,7 +488,7 @@ export class Replayer {
         }
         const target = mirror.getNode(d.id);
         if (!target) {
-          return this.warnTargetNotFound(d, d.id);
+          return this.warnNodeNotFound(d, d.id);
         }
         if ((target as Node) === this.iframe.contentDocument) {
           this.iframe.contentWindow!.scrollTo({
@@ -514,7 +527,7 @@ export class Replayer {
         }
         const target = mirror.getNode(d.id);
         if (!target) {
-          return this.warnTargetNotFound(d, d.id);
+          return this.warnNodeNotFound(d, d.id);
         }
         try {
           ((target as Node) as HTMLInputElement).checked = d.isChecked;
@@ -590,14 +603,10 @@ export class Replayer {
     this.noramlSpeed = -1;
   }
 
-  private warnTargetNotFound(d: incrementalData, id: number) {
+  private warnNodeNotFound(d: incrementalData, id: number) {
     if (!this.config.showWarning) {
       return;
     }
-    console.warn(
-      REPLAY_CONSOLE_PREFIX,
-      `target with id '${id}' not found in`,
-      d,
-    );
+    console.warn(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' not found in`, d);
   }
 }

src/types.ts

@@ -136,8 +136,7 @@ export type attributeMutation = {
 };
 
 export type removedNodeMutation = {
-  parentId?: number;
-  parentNode?: Node;
+  parentId: number;
   id: number;
 };
 

test/__snapshots__/integration.test.ts.snap

@@ -2194,10 +2194,6 @@ exports[`select2 1`] = `
         {
           \\"parentId\\": 25,
           \\"id\\": 36
-        },
-        {
-          \\"parentId\\": 45,
-          \\"id\\": 46
         }
       ],
       \\"adds\\": [