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.

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

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

Merged
merged 2 commits into from
Jan 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 12 additions & 11 deletions src/record/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
47 changes: 28 additions & 19 deletions src/replay/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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);
},
Expand All @@ -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:
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
3 changes: 1 addition & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ export type attributeMutation = {
};

export type removedNodeMutation = {
parentId?: number;
parentNode?: Node;
parentId: number;
id: number;
};

Expand Down
4 changes: 0 additions & 4 deletions test/__snapshots__/integration.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2194,10 +2194,6 @@ exports[`select2 1`] = `
{
\\"parentId\\": 25,
\\"id\\": 36
},
{
\\"parentId\\": 45,
\\"id\\": 46
}
],
\\"adds\\": [
Expand Down