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

Masking: Avoid the repeated calls to closest when recursing through the DOM #1349

Merged
merged 10 commits into from
Nov 24, 2023
6 changes: 6 additions & 0 deletions .changeset/thin-vans-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'rrweb-snapshot': patch
'rrweb': patch
---

Snapshot performance when masking text: Avoid the repeated calls to `closest` when recursing through the DOM
63 changes: 39 additions & 24 deletions packages/rrweb-snapshot/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
export function ignoreAttribute(
tagName: string,
name: string,
_value: unknown,

Check warning on line 255 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L255

[@typescript-eslint/no-unused-vars] '_value' is defined but never used.
): boolean {
return (tagName === 'video' || tagName === 'audio') && name === 'autoplay';
}
Expand Down Expand Up @@ -310,24 +310,29 @@
node: Node,
maskTextClass: string | RegExp,
maskTextSelector: string | null,
checkAncestors: boolean,
): boolean {
try {
const el: HTMLElement | null =
node.nodeType === node.ELEMENT_NODE
? (node as HTMLElement)
: node.parentElement;
if (el === null) return false;

if (typeof maskTextClass === 'string') {
if (el.classList.contains(maskTextClass)) return true;
if (el.closest(`.${maskTextClass}`)) return true;
if (checkAncestors) {
if (el.closest(`.${maskTextClass}`)) return true;
} else {
if (el.classList.contains(maskTextClass)) return true;
}
} else {
if (classMatchesRegex(el, maskTextClass, true)) return true;
if (classMatchesRegex(el, maskTextClass, checkAncestors)) return true;
}

if (maskTextSelector) {
if (el.matches(maskTextSelector)) return true;
if (el.closest(maskTextSelector)) return true;
if (checkAncestors) {
if (el.closest(maskTextSelector)) return true;
} else {
if (el.matches(maskTextSelector)) return true;
}
}
} catch (e) {
//
Expand Down Expand Up @@ -385,7 +390,7 @@
iframeEl.addEventListener('load', listener);
}

function isStylesheetLoaded(link: HTMLLinkElement) {

Check warning on line 393 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L393

[@typescript-eslint/no-unused-vars] 'isStylesheetLoaded' is defined but never used.
if (!link.getAttribute('href')) return true; // nothing to load
return link.sheet !== null;
}
Expand Down Expand Up @@ -426,8 +431,7 @@
mirror: Mirror;
blockClass: string | RegExp;
blockSelector: string | null;
maskTextClass: string | RegExp;
maskTextSelector: string | null;
needsMask: boolean | undefined;
inlineStylesheet: boolean;
maskInputOptions: MaskInputOptions;
maskTextFn: MaskTextFn | undefined;
Expand All @@ -447,8 +451,7 @@
mirror,
blockClass,
blockSelector,
maskTextClass,
maskTextSelector,
needsMask,
inlineStylesheet,
maskInputOptions = {},
maskTextFn,
Expand Down Expand Up @@ -500,8 +503,7 @@
});
case n.TEXT_NODE:
return serializeTextNode(n as Text, {
maskTextClass,
maskTextSelector,
needsMask,
maskTextFn,
rootId,
});
Expand Down Expand Up @@ -531,13 +533,12 @@
function serializeTextNode(
n: Text,
options: {
maskTextClass: string | RegExp;
maskTextSelector: string | null;
needsMask: boolean | undefined;
maskTextFn: MaskTextFn | undefined;
rootId: number | undefined;
},
): serializedNode {
const { maskTextClass, maskTextSelector, maskTextFn, rootId } = options;
const { needsMask, maskTextFn, rootId } = options;
// The parent node may not be a html element which has a tagName attribute.
// So just let it be undefined which is ok in this use case.
const parentTagName = n.parentNode && (n.parentNode as HTMLElement).tagName;
Expand All @@ -554,7 +555,7 @@
// So we'll be conservative and keep textContent as-is.
} else if ((n.parentNode as HTMLStyleElement).sheet?.cssRules) {
textContent = stringifyStylesheet(
(n.parentNode as HTMLStyleElement).sheet!,

Check warning on line 558 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L558

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
);
}
} catch (err) {
Expand All @@ -568,12 +569,7 @@
if (isScript) {
textContent = 'SCRIPT_PLACEHOLDER';
}
if (
!isStyle &&
!isScript &&
textContent &&
needMaskingText(n, maskTextClass, maskTextSelector)
) {
if (!isStyle && !isScript && textContent && needsMask) {
textContent = maskTextFn
? maskTextFn(textContent, n.parentElement)
: textContent.replace(/[\S]/g, '*');
Expand Down Expand Up @@ -648,7 +644,7 @@
if (cssText) {
delete attributes.rel;
delete attributes.href;
attributes._cssText = absoluteToStylesheet(cssText, stylesheet!.href!);

Check warning on line 647 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L647

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.

Check warning on line 647 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L647

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
}
}
// dynamic stylesheet
Expand Down Expand Up @@ -742,10 +738,10 @@
const recordInlineImage = () => {
image.removeEventListener('load', recordInlineImage);
try {
canvasService!.width = image.naturalWidth;

Check warning on line 741 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L741

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
canvasService!.height = image.naturalHeight;

Check warning on line 742 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L742

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
canvasCtx!.drawImage(image, 0, 0);

Check warning on line 743 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L743

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
attributes.rr_dataURL = canvasService!.toDataURL(

Check warning on line 744 in packages/rrweb-snapshot/src/snapshot.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/snapshot.ts#L744

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
dataURLOptions.type,
dataURLOptions.quality,
);
Expand Down Expand Up @@ -935,6 +931,7 @@
inlineStylesheet: boolean;
newlyAddedElement?: boolean;
maskInputOptions?: MaskInputOptions;
needsMask?: boolean;
maskTextFn: MaskTextFn | undefined;
maskInputFn: MaskInputFn | undefined;
slimDOMOptions: SlimDOMOptions;
Expand Down Expand Up @@ -980,14 +977,29 @@
keepIframeSrcFn = () => false,
newlyAddedElement = false,
} = options;
let { needsMask } = options;
let { preserveWhiteSpace = true } = options;

if (
!needsMask &&
n.childNodes // we can avoid the check on leaf elements, as masking is applied to child text nodes only
) {
// perf: if needsMask = true, children won't also need to check
const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors
needsMask = needMaskingText(
n as Element,
maskTextClass,
maskTextSelector,
checkAncestors,
);
}

const _serializedNode = serializeNode(n, {
doc,
mirror,
blockClass,
blockSelector,
maskTextClass,
maskTextSelector,
needsMask,
inlineStylesheet,
maskInputOptions,
maskTextFn,
Expand Down Expand Up @@ -1058,6 +1070,7 @@
mirror,
blockClass,
blockSelector,
needsMask,
maskTextClass,
maskTextSelector,
skipChild,
Expand Down Expand Up @@ -1118,6 +1131,7 @@
mirror,
blockClass,
blockSelector,
needsMask,
maskTextClass,
maskTextSelector,
skipChild: false,
Expand Down Expand Up @@ -1165,6 +1179,7 @@
mirror,
blockClass,
blockSelector,
needsMask,
maskTextClass,
maskTextSelector,
skipChild: false,
Expand Down
1 change: 1 addition & 0 deletions packages/rrweb/src/record/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,13 @@
};

while (this.mapRemoves.length) {
this.mirror.removeNodeFromMap(this.mapRemoves.shift()!);

Check warning on line 347 in packages/rrweb/src/record/mutation.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb/src/record/mutation.ts#L347

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
}

for (const n of this.movedSet) {
if (
isParentRemoved(this.removes, n, this.mirror) &&
!this.movedSet.has(n.parentNode!)

Check warning on line 353 in packages/rrweb/src/record/mutation.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb/src/record/mutation.ts#L353

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
) {
continue;
}
Expand Down Expand Up @@ -515,6 +515,7 @@
m.target,
this.maskTextClass,
this.maskTextSelector,
true, // checkAncestors
) && value
? this.maskTextFn
? this.maskTextFn(value, closestElementOfNode(m.target))
Expand Down
Loading
Loading