From c18626bf5ae31f1abc6d5268651bd3d8696cc610 Mon Sep 17 00:00:00 2001 From: Yanzhen Yu Date: Wed, 1 Apr 2026 12:00:00 +0800 Subject: [PATCH] fix mutation filter in recorder and change the order of apply mutations in replayer This change include two critical fix for both recorder and replayer. In the recorder, previously we filter text and attribute mutations by check its target id, but this was a wrong approach because removed node also has id at the callback moment. We corrected this by checking whether the mirror map still has the target id in its keys. In the replayer side, the issue was we got exceptions when calling insertBefore which says the ref node was not the child node of the caller node. This will happen when the previous or next sibling has been removed in the same callback but the previousId or nextId was recorded. After apply remove node mutations before add node mutations, we can make sure the removed siblings will not exist in the mirror map when apply add node mutations. When we get node from mirror map with an removed id, we will get null and pass it to insertBefore which is valid. As a side note, this apply order is safe because we ensured all the remove node mutations do not include removing newly added nodes in the same callback. --- src/record/observer.ts | 8 ++++---- src/replay/index.ts | 31 ++++++++++++++++--------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/record/observer.ts b/src/record/observer.ts index f0b58de8..0a7de203 100644 --- a/src/record/observer.ts +++ b/src/record/observer.ts @@ -159,15 +159,15 @@ function initMutationObserver(cb: mutationCallBack): MutationObserver { id: mirror.getId(text.node as INode), value: text.value, })) - // text mutation without ID means the target node has been removed - .filter(text => text.id), + // text mutation's id was not in the mirror map means the target node has been removed + .filter(text => mirror.has(text.id)), attributes: attributes .map(attribute => ({ id: mirror.getId(attribute.node as INode), attributes: attribute.attributes, })) - // attribute mutation without ID means the target node has been removed - .filter(attribute => attribute.id), + // attribute mutation's id was not in the mirror map means the target node has been removed + .filter(attribute => mirror.has(attribute.id)), removes, adds, }); diff --git a/src/replay/index.ts b/src/replay/index.ts index ff057447..10bcd8ed 100644 --- a/src/replay/index.ts +++ b/src/replay/index.ts @@ -175,6 +175,20 @@ export class Replayer { private applyIncremental(d: incrementalData, isSync: boolean) { switch (d.source) { case IncrementalSource.Mutation: { + d.removes.forEach(mutation => { + const target = mirror.getNode(mutation.id); + if (!target) { + return; + } + const parent = (mirror.getNode( + mutation.parentId!, + ) as Node) as Element; + // target may be removed with its parents before + mirror.removeNodeFromMap(target); + if (parent) { + parent.removeChild(target); + } + }); d.adds.forEach(mutation => { const target = buildNodeWithSN( mutation.node, @@ -184,12 +198,10 @@ export class Replayer { ) as Node; const parent = (mirror.getNode(mutation.parentId) as Node) as Element; if (mutation.nextId) { - const next = (mirror.getNode(mutation.nextId) as Node) as Element; + const next = mirror.getNode(mutation.nextId) as Node; parent.insertBefore(target, next); } else if (mutation.previousId) { - const previous = (mirror.getNode( - mutation.previousId, - ) as Node) as Element; + const previous = mirror.getNode(mutation.previousId) as Node; parent.insertBefore(target, previous.nextSibling); } else { parent.appendChild(target); @@ -212,17 +224,6 @@ export class Replayer { } } }); - d.removes.forEach(mutation => { - const target = (mirror.getNode(mutation.id) as Node) as Element; - const parent = (mirror.getNode( - mutation.parentId!, - ) as Node) as Element; - // target may be removed with its parents before - if (parent) { - parent.removeChild(target); - } - delete mirror.map[mutation.id]; - }); break; } case IncrementalSource.MouseMove: