Add more concrete "node not found" warnings (#620)
* Show whether a node was not found because it was never there, or because some other mutation already removed it * Be more careful applying mutations as replay-stopping things can go wrong, e.g. node removal parent belongs to a different FullSnapshot and has e.g. a text nodeType * Don't warn when a modified node has been removed in the same mutation - or the parent node for a removed node has been already removed
This commit is contained in:
@@ -769,7 +769,11 @@ export class Replayer {
|
|||||||
d.attributes.forEach((m) => this.treeIndex.attribute(m));
|
d.attributes.forEach((m) => this.treeIndex.attribute(m));
|
||||||
d.removes.forEach((m) => this.treeIndex.remove(m, this.mirror));
|
d.removes.forEach((m) => this.treeIndex.remove(m, this.mirror));
|
||||||
}
|
}
|
||||||
|
try {
|
||||||
this.applyMutation(d, isSync);
|
this.applyMutation(d, isSync);
|
||||||
|
} catch (error) {
|
||||||
|
this.warn(`Exception in mutation ${error.message || error}`, d);
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
case IncrementalSource.Drag:
|
case IncrementalSource.Drag:
|
||||||
@@ -1056,6 +1060,10 @@ export class Replayer {
|
|||||||
d.removes.forEach((mutation) => {
|
d.removes.forEach((mutation) => {
|
||||||
const target = this.mirror.getNode(mutation.id);
|
const target = this.mirror.getNode(mutation.id);
|
||||||
if (!target) {
|
if (!target) {
|
||||||
|
if (d.removes.find((r) => r.id === mutation.parentId)) {
|
||||||
|
// no need to warn, parent was already removed
|
||||||
|
return;
|
||||||
|
}
|
||||||
return this.warnNodeNotFound(d, mutation.id);
|
return this.warnNodeNotFound(d, mutation.id);
|
||||||
}
|
}
|
||||||
let parent: INode | null | ShadowRoot = this.mirror.getNode(
|
let parent: INode | null | ShadowRoot = this.mirror.getNode(
|
||||||
@@ -1070,20 +1078,28 @@ export class Replayer {
|
|||||||
// target may be removed with its parents before
|
// target may be removed with its parents before
|
||||||
this.mirror.removeNodeFromMap(target);
|
this.mirror.removeNodeFromMap(target);
|
||||||
if (parent) {
|
if (parent) {
|
||||||
|
let realTarget = null;
|
||||||
const realParent =
|
const realParent =
|
||||||
'__sn' in parent ? this.fragmentParentMap.get(parent) : undefined;
|
'__sn' in parent ? this.fragmentParentMap.get(parent) : undefined;
|
||||||
if (realParent && realParent.contains(target)) {
|
if (realParent && realParent.contains(target)) {
|
||||||
realParent.removeChild(target);
|
parent = realParent;
|
||||||
} else if (this.fragmentParentMap.has(target)) {
|
} else if (this.fragmentParentMap.has(target)) {
|
||||||
/**
|
/**
|
||||||
* the target itself is a fragment document and it's not in the dom
|
* the target itself is a fragment document and it's not in the dom
|
||||||
* so we should remove the real target from its parent
|
* so we should remove the real target from its parent
|
||||||
*/
|
*/
|
||||||
const realTarget = this.fragmentParentMap.get(target)!;
|
realTarget = this.fragmentParentMap.get(target)!;
|
||||||
parent.removeChild(realTarget);
|
|
||||||
this.fragmentParentMap.delete(target);
|
this.fragmentParentMap.delete(target);
|
||||||
} else {
|
target = realTarget;
|
||||||
|
}
|
||||||
|
try {
|
||||||
parent.removeChild(target);
|
parent.removeChild(target);
|
||||||
|
} catch (error) {
|
||||||
|
if (error instanceof DOMException) {
|
||||||
|
this.warn('parent could not remove child in mutation', parent, realParent, target, realTarget , d);
|
||||||
|
} else {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -1290,6 +1306,10 @@ export class Replayer {
|
|||||||
d.texts.forEach((mutation) => {
|
d.texts.forEach((mutation) => {
|
||||||
let target = this.mirror.getNode(mutation.id);
|
let target = this.mirror.getNode(mutation.id);
|
||||||
if (!target) {
|
if (!target) {
|
||||||
|
if (d.removes.find((r) => r.id === mutation.id)) {
|
||||||
|
// no need to warn, element was already removed
|
||||||
|
return;
|
||||||
|
}
|
||||||
return this.warnNodeNotFound(d, mutation.id);
|
return this.warnNodeNotFound(d, mutation.id);
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
@@ -1303,6 +1323,10 @@ export class Replayer {
|
|||||||
d.attributes.forEach((mutation) => {
|
d.attributes.forEach((mutation) => {
|
||||||
let target = this.mirror.getNode(mutation.id);
|
let target = this.mirror.getNode(mutation.id);
|
||||||
if (!target) {
|
if (!target) {
|
||||||
|
if (d.removes.find((r) => r.id === mutation.id)) {
|
||||||
|
// no need to warn, element was already removed
|
||||||
|
return;
|
||||||
|
}
|
||||||
return this.warnNodeNotFound(d, mutation.id);
|
return this.warnNodeNotFound(d, mutation.id);
|
||||||
}
|
}
|
||||||
if (this.fragmentParentMap.has(target)) {
|
if (this.fragmentParentMap.has(target)) {
|
||||||
@@ -1584,7 +1608,11 @@ export class Replayer {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private warnNodeNotFound(d: incrementalData, id: number) {
|
private warnNodeNotFound(d: incrementalData, id: number) {
|
||||||
this.warn(`Node with id '${id}' not found in`, d);
|
if (this.treeIndex.removeIdSet.has(id)) {
|
||||||
|
this.warn(`Node with id '${id}' was previously removed. `, d);
|
||||||
|
} else {
|
||||||
|
this.warn(`Node with id '${id}' not found. `, d);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private warnCanvasMutationFailed(
|
private warnCanvasMutationFailed(
|
||||||
@@ -1602,7 +1630,11 @@ export class Replayer {
|
|||||||
* is microtask, so events fired on a removed DOM may emit
|
* is microtask, so events fired on a removed DOM may emit
|
||||||
* snapshots in the reverse order.
|
* snapshots in the reverse order.
|
||||||
*/
|
*/
|
||||||
this.debug(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' not found in`, d);
|
if (this.treeIndex.removeIdSet.has(id)) {
|
||||||
|
this.debug(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' was previously removed. `, d);
|
||||||
|
} else {
|
||||||
|
this.debug(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' not found. `, d);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private warn(...args: Parameters<typeof console.warn>) {
|
private warn(...args: Parameters<typeof console.warn>) {
|
||||||
|
|||||||
Reference in New Issue
Block a user