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

Merge changes from master in to pendo-main #27

Closed
wants to merge 5 commits into from
Closed
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
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 @@ -335,6 +335,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
45 changes: 26 additions & 19 deletions packages/rrweb/src/record/mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
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 @@ -362,13 +363,13 @@
};

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

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

View workflow job for this annotation

GitHub Actions / ESLint Check and Report Upload

Forbidden non-null assertion

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

View workflow job for this annotation

GitHub Actions / ESLint Check and Report Upload

Forbidden non-null assertion

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

View workflow job for this annotation

GitHub Actions / ESLint Check and Report Upload

Forbidden non-null assertion

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

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

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

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

View workflow job for this annotation

GitHub Actions / ESLint Check and Report Upload

Forbidden non-null assertion

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
) {
continue;
}
Expand All @@ -378,7 +379,7 @@
for (const n of this.addedSet) {
if (
!isAncestorInSet(this.droppedSet, n) &&
!isParentRemoved(this.removes, n, this.mirror)
!isParentRemoved(this.removesSubTreeCache, n, this.mirror)
) {
pushAdd(n);
} else if (isAncestorInSet(this.movedSet, n)) {
Expand Down Expand Up @@ -514,6 +515,7 @@
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 @@ -740,6 +742,7 @@
? true
: undefined,
});
processRemoves(n, this.removesSubTreeCache);
}
this.mapRemoves.push(n);
});
Expand Down Expand Up @@ -803,29 +806,33 @@
dom.childNodes(n).forEach((childN) => deepDelete(addsSet, childN));
}

function isParentRemoved(
removes: removedNodeMutation[],
n: Node,
mirror: Mirror,
): boolean {
if (removes.length === 0) return false;
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(
removes: removedNodeMutation[],
removes: Set<Node>,
n: Node,
mirror: Mirror,
_mirror: Mirror,

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

View workflow job for this annotation

GitHub Actions / ESLint Check and Report Upload

'_mirror' is defined but never used

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

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

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

[@typescript-eslint/no-unused-vars] '_mirror' is defined but never used.
): boolean {
let node: ParentNode | null = dom.parentNode(n);
while (node) {
const parentId = mirror.getId(node);
if (removes.some((r) => r.id === parentId)) {
return true;
}
node = dom.parentNode(node);
}
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
Loading