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
This commit is contained in:
yz-yu
2019-01-21 19:18:51 +08:00
committed by GitHub
parent c0ec1bd49a
commit 3daedfa284
4 changed files with 41 additions and 36 deletions

View File

@@ -102,6 +102,8 @@ 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;
}
@@ -109,17 +111,24 @@ function initMutationObserver(cb: mutationCallBack): MutationObserver {
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) {

View File

@@ -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);
}
}

View File

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

View File

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