Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Node not found warnings #620

Merged
merged 3 commits into from
Jul 19, 2021
Merged

Node not found warnings #620

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8b2c77
Show whether a node was not found because it was never there, or beca…
eoghanmurray Jul 1, 2021
c999937
Be more careful applying mutations as replay-stopping things can go w…
eoghanmurray Jul 1, 2021
853a51e
Don't warn when a modified node has been removed in the same mutation…
eoghanmurray Jul 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions src/replay/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,11 @@ export class Replayer {
d.attributes.forEach((m) => this.treeIndex.attribute(m));
d.removes.forEach((m) => this.treeIndex.remove(m, this.mirror));
}
this.applyMutation(d, isSync);
try {
this.applyMutation(d, isSync);
} catch (error) {
this.warn(`Exception in mutation ${error.message || error}`, d);
}
break;
}
case IncrementalSource.Drag:
Expand Down Expand Up @@ -1055,6 +1059,10 @@ export class Replayer {
d.removes.forEach((mutation) => {
const target = this.mirror.getNode(mutation.id);
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);
}
let parent: INode | null | ShadowRoot = this.mirror.getNode(
Expand All @@ -1069,20 +1077,28 @@ export class Replayer {
// target may be removed with its parents before
this.mirror.removeNodeFromMap(target);
if (parent) {
let realTarget = null;
const realParent =
'__sn' in parent ? this.fragmentParentMap.get(parent) : undefined;
if (realParent && realParent.contains(target)) {
realParent.removeChild(target);
parent = realParent;
} else if (this.fragmentParentMap.has(target)) {
/**
* the target itself is a fragment document and it's not in the dom
* so we should remove the real target from its parent
*/
const realTarget = this.fragmentParentMap.get(target)!;
parent.removeChild(realTarget);
realTarget = this.fragmentParentMap.get(target)!;
this.fragmentParentMap.delete(target);
} else {
target = realTarget;
}
try {
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;
}
}
}
});
Expand Down Expand Up @@ -1289,6 +1305,10 @@ export class Replayer {
d.texts.forEach((mutation) => {
let target = this.mirror.getNode(mutation.id);
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);
}
/**
Expand All @@ -1302,6 +1322,10 @@ export class Replayer {
d.attributes.forEach((mutation) => {
let target = this.mirror.getNode(mutation.id);
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);
}
if (this.fragmentParentMap.has(target)) {
Expand Down Expand Up @@ -1569,7 +1593,11 @@ export class Replayer {
}

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(
Expand All @@ -1587,7 +1615,11 @@ export class Replayer {
* is microtask, so events fired on a removed DOM may emit
* 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>) {
Expand Down