Skip to content

Commit

Permalink
Masking: Avoid the repeated calls to closest when recursing through…
Browse files Browse the repository at this point in the history
… the DOM (#1349)

* masking performance: avoid the repeated calls to `closest` when recursing through the DOM
 - needsMask===true means that an ancestor has tested positively for masking, and so this node and all descendents should be masked
 - needsMask===false means that no ancestors have tested positively for masking, we should check each node encountered
 - needsMask===undefined means that we don't know whether ancestors are masked or not (e.g. after a mutation) and should look up the tree
* Add tests including an explicit characterData mutation tests 
* Further performance improvement: avoid calls to `el.matches` when on a leaf node, e.g. a `<br/>`
---------

Authored-by: eoghanmurray <eoghan@getthere.ie>
Based on initial PR #1338 by Alexey Babik <alexey.babik@noibu.com>
  • Loading branch information
eoghanmurray authored Nov 24, 2023
1 parent 8aea5b0 commit 07ac5c9
Show file tree
Hide file tree
Showing 6 changed files with 323 additions and 24 deletions.
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 @@ -310,24 +310,29 @@ export function needMaskingText(
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 @@ -426,8 +431,7 @@ function serializeNode(
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 @@ function serializeNode(
mirror,
blockClass,
blockSelector,
maskTextClass,
maskTextSelector,
needsMask,
inlineStylesheet,
maskInputOptions = {},
maskTextFn,
Expand Down Expand Up @@ -500,8 +503,7 @@ function serializeNode(
});
case n.TEXT_NODE:
return serializeTextNode(n as Text, {
maskTextClass,
maskTextSelector,
needsMask,
maskTextFn,
rootId,
});
Expand Down Expand Up @@ -531,13 +533,12 @@ function getRootId(doc: Document, mirror: Mirror): number | undefined {
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 Down Expand Up @@ -568,12 +569,7 @@ function serializeTextNode(
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 @@ -935,6 +931,7 @@ export function serializeNodeWithId(
inlineStylesheet: boolean;
newlyAddedElement?: boolean;
maskInputOptions?: MaskInputOptions;
needsMask?: boolean;
maskTextFn: MaskTextFn | undefined;
maskInputFn: MaskInputFn | undefined;
slimDOMOptions: SlimDOMOptions;
Expand Down Expand Up @@ -980,14 +977,29 @@ export function serializeNodeWithId(
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 @@ export function serializeNodeWithId(
mirror,
blockClass,
blockSelector,
needsMask,
maskTextClass,
maskTextSelector,
skipChild,
Expand Down Expand Up @@ -1118,6 +1131,7 @@ export function serializeNodeWithId(
mirror,
blockClass,
blockSelector,
needsMask,
maskTextClass,
maskTextSelector,
skipChild: false,
Expand Down Expand Up @@ -1165,6 +1179,7 @@ export function serializeNodeWithId(
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 @@ -515,6 +515,7 @@ export default class MutationBuffer {
m.target,
this.maskTextClass,
this.maskTextSelector,
true, // checkAncestors
) && value
? this.maskTextFn
? this.maskTextFn(value, closestElementOfNode(m.target))
Expand Down
Loading

0 comments on commit 07ac5c9

Please sign in to comment.