Skip to content

Commit

Permalink
Merge pull request #28 from pendo-io/jg-merge-master-to-pendo-main
Browse files Browse the repository at this point in the history
Merge master in to pendo-main
  • Loading branch information
jaj1014 authored Oct 23, 2024
2 parents 0de8fe6 + acb3d53 commit ec7d7e6
Show file tree
Hide file tree
Showing 14 changed files with 157 additions and 82 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-penguins-sin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"rrdom": patch
---

Ignore invalid DOM attributes when diffing
2 changes: 2 additions & 0 deletions .changeset/metal-mugs-mate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
5 changes: 5 additions & 0 deletions .changeset/perfect-dolls-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"rrweb-snapshot": patch
---

fix dimensions for blocked element not being applied
6 changes: 6 additions & 0 deletions .changeset/simplifify-hover-replacement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"rrweb-snapshot": patch
"rrweb": patch
---

Slight simplification to how we replace :hover after #1458
5 changes: 5 additions & 0 deletions .changeset/swift-pots-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"rrweb": minor
---

Optimize isParentRemoved check
6 changes: 3 additions & 3 deletions docs/recipes/optimize-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ rrweb.record({
rrweb.record({
emit(event) {},
sampling: {
// Configure which kins of mouse interaction should be recorded
// Configure which kinds of mouse interaction should be recorded
mouseInteraction: {
MouseUp: false,
MouseDown: false,
Expand Down Expand Up @@ -78,7 +78,7 @@ import { pack } from '@rrweb/packer';

rrweb.record({
emit(event) {},
packFn: rrweb.pack,
packFn: pack,
});
```

Expand All @@ -88,7 +88,7 @@ And you need to pass packer.unpack as the `unpackFn` in replaying.
import { unpack } from '@rrweb/packer';

const replayer = new rrweb.Replayer(events, {
unpackFn: rrweb.unpack,
unpackFn: unpack,
});
```

Expand Down
11 changes: 10 additions & 1 deletion packages/rrdom/src/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,16 @@ function diffProps(
}
};
} else if (newTree.tagName === 'IFRAME' && name === 'srcdoc') continue;
else oldTree.setAttribute(name, newValue);
else {
try {
oldTree.setAttribute(name, newValue);
} catch (err) {
// We want to continue diffing so we quietly catch
// this exception. Otherwise, this can throw and bubble up to
// the `ReplayerEvents.Flush` listener and break rendering
console.warn(err);
}
}
}

for (const { name } of Array.from(oldAttributes))
Expand Down
26 changes: 26 additions & 0 deletions packages/rrdom/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,32 @@ describe('diff algorithm for rrdom', () => {
expect((node as Node as HTMLElement).className).toBe('node');
});

it('ignores invalid attributes', () => {
const tagName = 'DIV';
const node = document.createElement(tagName);
const sn = Object.assign({}, elementSn, {
attributes: { '@click': 'foo' },
tagName,
});
mirror.add(node, sn);

const rrDocument = new RRDocument();
const rrNode = rrDocument.createElement(tagName);
const sn2 = Object.assign({}, elementSn, {
attributes: { '@click': 'foo' },
tagName,
});
rrDocument.mirror.add(rrNode, sn2);

rrNode.attributes = { id: 'node1', class: 'node', '@click': 'foo' };
diff(node, rrNode, replayer);
expect((node as Node as HTMLElement).id).toBe('node1');
expect((node as Node as HTMLElement).className).toBe('node');
expect('@click' in (node as Node as HTMLElement)).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
warn.mockClear();
});

it('can update exist properties', () => {
const tagName = 'DIV';
const node = document.createElement(tagName);
Expand Down
28 changes: 28 additions & 0 deletions packages/rrdom/test/virtual-dom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as puppeteer from 'puppeteer';
import { vi } from 'vitest';
import { JSDOM } from 'jsdom';
import {
buildNodeWithSN,
cdataNode,
commentNode,
documentNode,
Expand Down Expand Up @@ -207,6 +208,33 @@ describe('RRDocument for browser environment', () => {
expect((rrNode as RRElement).tagName).toEqual('SHADOWROOT');
expect(rrNode).toBe(parentRRNode.shadowRoot);
});

it('can rebuild blocked element with correct dimensions', () => {
// @ts-expect-error Testing buildNodeWithSN with rr elements
const node = buildNodeWithSN(
{
id: 1,
tagName: 'svg',
type: NodeType.Element,
isSVG: true,
attributes: {
rr_width: '50px',
rr_height: '50px',
},
childNodes: [],
},
{
// @ts-expect-error
doc: new RRDocument(),
mirror,
blockSelector: '*',
slimDOMOptions: {},
},
) as RRElement;

expect(node.style.width).toBe('50px');
expect(node.style.height).toBe('50px');
});
});

describe('create a RRDocument from a html document', () => {
Expand Down
55 changes: 3 additions & 52 deletions packages/rrweb-snapshot/src/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const mediaSelectorPlugin: AcceptedPlugin = {
},
};

// Adapted from https://github.com/giuseppeg/postcss-pseudo-classes/blob/master/index.js
// Simplified from https://github.com/giuseppeg/postcss-pseudo-classes/blob/master/index.js
const pseudoClassPlugin: AcceptedPlugin = {
postcssPlugin: 'postcss-hover-classes',
prepare: function () {
Expand All @@ -28,58 +28,9 @@ const pseudoClassPlugin: AcceptedPlugin = {
return;
}
fixed.push(rule);

rule.selectors.forEach(function (selector) {
if (!selector.includes(':')) {
return;
}

const selectorParts = selector.replace(/\n/g, ' ').split(' ');
const pseudoedSelectorParts: string[] = [];

selectorParts.forEach(function (selectorPart) {
const pseudos = selectorPart.match(/::?([^:]+)/g);

if (!pseudos) {
pseudoedSelectorParts.push(selectorPart);
return;
}

const baseSelector = selectorPart.substr(
0,
selectorPart.length - pseudos.join('').length,
);

const classPseudos = pseudos.map(function (pseudo) {
const pseudoToCheck = pseudo.replace(/\(.*/g, '');
if (pseudoToCheck !== ':hover') {
return pseudo;
}

// Ignore pseudo-elements!
if (pseudo.match(/^::/)) {
return pseudo;
}

// Kill the colon
pseudo = pseudo.substr(1);

// Replace left and right parens
pseudo = pseudo.replace(/\(/g, '\\(');
pseudo = pseudo.replace(/\)/g, '\\)');

return '.' + '\\:' + pseudo;
});

pseudoedSelectorParts.push(baseSelector + classPseudos.join(''));
});

addSelector(pseudoedSelectorParts.join(' '));

function addSelector(newSelector: string) {
if (newSelector && newSelector !== selector) {
rule.selector += ',\n' + newSelector;
}
if (selector.includes(':hover')) {
rule.selector += ',\n' + selector.replace(/:hover/g, '.\\:hover');
}
});
},
Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb-snapshot/src/rebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,9 @@ function buildNode(
}

if (name === 'rr_width') {
(node as HTMLElement).style.width = value.toString();
(node as HTMLElement).style.setProperty('width', value.toString());
} else if (name === 'rr_height') {
(node as HTMLElement).style.height = value.toString();
(node as HTMLElement).style.setProperty('height', value.toString());
} else if (
name === 'rr_mediaCurrentTime' &&
typeof value === 'number'
Expand Down
10 changes: 9 additions & 1 deletion packages/rrweb-snapshot/test/css.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,18 @@ describe('css parser', () => {
describe('pseudoClassPlugin', () => {
it('parses nested commas in selectors correctly', () => {
const cssText =
'body > ul :is(li:not(:first-of-type) a:hover, li:not(:first-of-type).active a) {background: red;}';
'body > ul :is(li:not(:first-of-type) a.current, li:not(:first-of-type).active a) {background: red;}';
expect(parse(pseudoClassPlugin, cssText)).toEqual(cssText);
});

it("doesn't ignore :hover within :is brackets", () => {
const cssText =
'body > ul :is(li:not(:first-of-type) a:hover, li:not(:first-of-type).active a) {background: red;}';
expect(parse(pseudoClassPlugin, cssText))
.toEqual(`body > ul :is(li:not(:first-of-type) a:hover, li:not(:first-of-type).active a),
body > ul :is(li:not(:first-of-type) a.\\:hover, li:not(:first-of-type).active a) {background: red;}`);
});

it('should parse selector with comma nested inside ()', () => {
const cssText =
'[_nghost-ng-c4172599085]:not(.fit-content).aim-select:hover:not(:disabled, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--disabled, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--invalid, [_nghost-ng-c4172599085]:not(.fit-content).aim-select--active) { border-color: rgb(84, 84, 84); }';
Expand Down
26 changes: 26 additions & 0 deletions packages/rrweb-snapshot/test/rebuild.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,32 @@ describe('rebuild', function () {
});
});

describe('rr_width/rr_height', function () {
it('rebuild blocked element with correct dimensions', function () {
const node = buildNodeWithSN(
{
id: 1,
tagName: 'svg',
type: NodeType.Element,
isSVG: true,
attributes: {
rr_width: '50px',
rr_height: '50px',
},
childNodes: [],
},
{
doc: document,
mirror,
hackCss: false,
cache,
},
) as HTMLDivElement;
expect(node.style.width).toBe('50px');
expect(node.style.height).toBe('50px');
});
});

describe('shadowDom', function () {
it('rebuild shadowRoot without siblings', function () {
const node = buildNodeWithSN(
Expand Down
50 changes: 27 additions & 23 deletions packages/rrweb/src/record/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ export default class MutationBuffer {
private attributes: attributeCursor[] = [];
private attributeMap = new WeakMap<Node, attributeCursor>();
private removes: removedNodeMutation[] = [];
private removesMap = new Map<number, number>();
private mapRemoves: Node[] = [];

private movedMap: Record<string, true> = {};
Expand All @@ -201,6 +200,7 @@ export default class MutationBuffer {
private addedSet = new Set<Node>();
private movedSet = new Set<Node>();
private droppedSet = new Set<Node>();
private removesSubTreeCache = new Set<Node>();

private mutationCb: observerParam['mutationCb'];
private blockClass: observerParam['blockClass'];
Expand Down Expand Up @@ -399,7 +399,7 @@ export default class MutationBuffer {

for (const n of this.movedSet) {
if (
isParentRemoved(this.removesMap, n, this.mirror) &&
isParentRemoved(this.removesSubTreeCache, n, this.mirror) &&
!this.movedSet.has(dom.parentNode(n)!)
) {
continue;
Expand All @@ -410,7 +410,7 @@ export default class MutationBuffer {
for (const n of this.addedSet) {
if (
!isAncestorInSet(this.droppedSet, n) &&
!isParentRemoved(this.removesMap, n, this.mirror)
!isParentRemoved(this.removesSubTreeCache, n, this.mirror)
) {
pushAdd(n);
} else if (isAncestorInSet(this.movedSet, n)) {
Expand Down Expand Up @@ -547,10 +547,10 @@ export default class MutationBuffer {
this.attributes = [];
this.attributeMap = new WeakMap<Node, attributeCursor>();
this.removes = [];
this.removesMap = new Map<number, number>();
this.addedSet = new Set<Node>();
this.movedSet = new Set<Node>();
this.droppedSet = new Set<Node>();
this.removesSubTreeCache = new Set<Node>();
this.movedMap = {};

this.mutationCb(payload);
Expand Down Expand Up @@ -785,7 +785,7 @@ export default class MutationBuffer {
? true
: undefined,
});
this.removesMap.set(nodeId, this.removes.length - 1);
processRemoves(n, this.removesSubTreeCache);
}
this.mapRemoves.push(n);
});
Expand Down Expand Up @@ -849,29 +849,33 @@ function deepDelete(addsSet: Set<Node>, n: Node) {
dom.childNodes(n).forEach((childN) => deepDelete(addsSet, childN));
}

function isParentRemoved(
removesMap: Map<number, number>,
n: Node,
mirror: Mirror,
): boolean {
if (removesMap.size === 0) return false;
return _isParentRemoved(removesMap, n, mirror);
function processRemoves(n: Node, cache: Set<Node>) {
const queue = [n];

while (queue.length) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const next = queue.pop()!;
if (cache.has(next)) continue;
cache.add(next);
dom.childNodes(next).forEach((n) => queue.push(n));
}

return;
}

function isParentRemoved(removes: Set<Node>, n: Node, mirror: Mirror): boolean {
if (removes.size === 0) return false;
return _isParentRemoved(removes, n, mirror);
}

function _isParentRemoved(
removesMap: Map<number, number>,
removes: Set<Node>,
n: Node,
mirror: Mirror,
_mirror: Mirror,
): boolean {
let node: ParentNode | null = n.parentNode;
while (node) {
const parentId = mirror.getId(node);
if (removesMap.has(parentId)) {
return true;
}
node = node.parentNode;
}
return false;
const node: ParentNode | null = dom.parentNode(n);
if (!node) return false;
return removes.has(node);
}

function isAncestorInSet(set: Set<Node>, n: Node): boolean {
Expand Down

0 comments on commit ec7d7e6

Please sign in to comment.