Skip to content

Commit

Permalink
Sync from source repo (#19)
Browse files Browse the repository at this point in the history
* Fix up the 'should replace the existing DOM nodes on iframe navigation with `isAttachIframe`' test (rrweb-io#1636)

- it was working for me when the test was run in isolation (`-t` option), but when the entire cross-origin-iframes test was run, the change of iframe contents didn't seem to happen in time

* [chore]: Update actions/upload-artifact to v4 (rrweb-io#1643)

* update actions/upload-artifact to v4

---------

Co-authored-by: Eoghan Murray <eoghan@getthere.ie>

* Fix a code path where masking could be skipped on textareas (rrweb-io#1599)

* Fixes rrweb-io#1596

* [chore] Cache yarn packages for CI (rrweb-io#1646)

* [chore] Cache yarn packages for CI

* Cache yarn in release.yml

* [chore] Update deprecated download artifact on CI (rrweb-io#1647)

* I'm merging even though ESLint is stlll failing in Github Actions as I believe it's running actions _without_ this PR applied yet

* Fix env puppeteer error in cross-origin-iframes.test.ts (rrweb-io#1629)

* chore(ci): track bundle size (rrweb-io#1630)

* chore(ci): track bundle size

---------

Co-authored-by: pauldambra <pauldambra@users.noreply.github.com>

* Fix adapt css with split (rrweb-io#1600)

Fix for rrweb-io#1575 where postcss was raising an exception

* adapt the entire CSS as a whole in one pass with postcss, rather than adapting each split part separately
* break up the postcss output again and assign to individual text nodes (kind of inverse of splitCssText at record side)
* impose an upper bound of 30 iterations on the substring searches to preempt possible pathological behavior
* add tests to demonstrate the scenario and prevent regression

More technical details:
* Fix algorithm; checks against `ix_end` within loop were incorrect when `ix_start` was bigger than zero.  
* Fix that length check against wrong array was causing 'should record style mutations with multiple child nodes and replay them correctly' test to fail. 
Note on last point: I haven't looked into things more deeply than that the test was complaining about missing .length after `replayer.pause(1000);`

* Warn instead of fail on exceptions thrown from postcss (rrweb-io#1580)

* postcss was introduced in rrweb-io#1458 for use within adaptCssForReplay
* rrweb-io#1600 fixes the main case where invalid css could be introduced when if valid css from the output of `sheet.cssRules` was split according to how it was split across text nodes of the <style>
* the guard introduced here is still useful as we likely in future will switch to capturing the raw stylesheet contents (both <style> and <link>), at which point we will be much less confident of getting valid css

* Fix splitCssText again (rrweb-io#1640)

Fixes a browser 'lock up' at record time due to a presence of large amounts of css in <style> elements, which are split over multiple text nodes, which triggers the new code added in rrweb-io#1437 (see that PR for full explanation of why this all exists).  rrweb-io#1437 was not written with performance in mind as it was believed to be an edge case, but things like Grammarly browser extension (rrweb-io#1603) among other scenarios were triggering pathological behavior, some of which was solved in rrweb-io#1615.
See also rrweb-io#1640 (comment) for further discussion.

* Fix the case when there are multiple matches and we end up not finding a unique one - just go with the best guess when there are many splits by looking at the previous chunk's size
* Also add '0px' -> '0' stylesheet normalization, which also fixes the sample problem in a different way
* Add new test and modify it so that it can trigger a failure in the absence of the '0px' normalization; there may be other unknown ways of triggering a similar bug, so ensure that the primary 'best guess' method doesn't suffer a regression
* Leverage the 'best guess' method so that we can quit after 100 iterations trying to find a unique substring; hopefully this bit along with the `iterLimit` already added will prevent any future pathological cases.

Failing example extracted from large files identified by Paul D'Ambra (Posthog) ... see comment from MartinWorkfully: PostHog/posthog-js#1668

* fix: move patch function into utils to improve bundling (rrweb-io#1631)

* fix: move patch function into utils to improve bundling

---------

Co-authored-by: pauldambra <pauldambra@users.noreply.github.com>
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>

---------

Co-authored-by: Eoghan Murray <eoghan@getthere.ie>
Co-authored-by: Kevin Townsend <11738094+kevinatown@users.noreply.github.com>
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>
Co-authored-by: Paul D'Ambra <paul@posthog.com>
Co-authored-by: pauldambra <pauldambra@users.noreply.github.com>
Co-authored-by: John Henry Gunther <jguntherenator@gmail.com>
  • Loading branch information
7 people authored Feb 7, 2025
1 parent 9e238e6 commit 4309de7
Show file tree
Hide file tree
Showing 26 changed files with 393 additions and 87 deletions.
5 changes: 5 additions & 0 deletions .changeset/angry-turtles-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"rrweb-snapshot": patch
---

Handle exceptions thrown from postcss when calling adaptCssForReplay
6 changes: 6 additions & 0 deletions .changeset/efficiently-splitCssText-1640.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"rrweb-snapshot": patch
"rrweb": patch
---

Improve performance of splitCssText for <style> elements with large css content - see #1603
6 changes: 6 additions & 0 deletions .changeset/fix-adapt-css.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"rrweb": patch
"rrweb-snapshot": patch
---

#1575 Fix that postcss could fall over when trying to process css content split arbitrarily
8 changes: 8 additions & 0 deletions .changeset/itchy-tables-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@rrweb/rrweb-plugin-console-record": patch
"@rrweb/record": patch
"rrweb": patch
"@rrweb/utils": patch
---

Move patch function into @rrweb/utils to improve bundling
2 changes: 2 additions & 0 deletions .changeset/shy-countries-rhyme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
8 changes: 8 additions & 0 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ jobs:
# run: PUPPETEER_EXECUTABLE_PATH=${{ steps.setup-chrome.outputs.chrome-path }} PUPPETEER_HEADLESS=true xvfb-run --server-args="-screen 0 1920x1080x24" yarn test
run: PUPPETEER_HEADLESS=true xvfb-run --server-args="-screen 0 1920x1080x24" yarn test

- name: Check bundle sizes
uses: preactjs/compressed-size-action@v2
with:
install-script: "yarn install --frozen-lockfile"
build-script: "build:all"
compression: "none"
pattern: "**/dist/*.{js,cjs,mjs,css}"

- name: Upload diff images to GitHub
uses: actions/upload-artifact@v4
if: failure()
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ dist
# for vite
vite.config.js.timestamp-*
vite.config.ts.timestamp-*

# bundle analysis files
*-bundle-analysis.html
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"markdownlint": "^0.25.1",
"markdownlint-cli": "^0.31.1",
"prettier": "2.8.4",
"rollup-plugin-visualizer": "^5.12.0",
"turbo": "^2.0.4",
"typescript": "^5.4.5"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/plugins/rrweb-plugin-console-record/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"puppeteer": "^20.9.0"
},
"peerDependencies": {
"rrweb": "^2.0.1-alpha.20"
"rrweb": "^2.0.1-alpha.20",
"@rrweb/utils": "^2.0.1-alpha.20"
}
}
4 changes: 2 additions & 2 deletions packages/plugins/rrweb-plugin-console-record/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { listenerHandler, RecordPlugin, IWindow } from '@rrweb/types';
import { utils } from 'rrweb';
import { patch } from '@rrweb/utils';
import { ErrorStackParser, StackFrame } from './error-stack-parser';
import { stringify } from './stringify';

Expand Down Expand Up @@ -183,7 +183,7 @@ function initLogObserver(
};
}
// replace the logger.{level}. return a restore function
return utils.patch(
return patch(
_logger,
level,
(original: (...args: Array<unknown>) => void) => {
Expand Down
3 changes: 2 additions & 1 deletion packages/record/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@
},
"dependencies": {
"@rrweb/types": "^2.0.1-alpha.20",
"rrweb": "^2.0.1-alpha.20"
"rrweb": "^2.0.1-alpha.20",
"@rrweb/utils": "^2.0.1-alpha.20"
},
"browserslist": [
"supports es6-class"
Expand Down
3 changes: 3 additions & 0 deletions packages/record/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
},
{
"path": "../rrweb"
},
{
"path": "../utils"
}
]
}
59 changes: 47 additions & 12 deletions packages/rrweb-snapshot/src/rebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,17 @@ export function adaptCssForReplay(cssText: string, cache: BuildCache): string {
const cachedStyle = cache?.stylesWithHoverClass.get(cssText);
if (cachedStyle) return cachedStyle;

const ast: { css: string } = postcss([
mediaSelectorPlugin,
pseudoClassPlugin,
]).process(cssText);
const result = ast.css;
let result = cssText;
try {
const ast: { css: string } = postcss([
mediaSelectorPlugin,
pseudoClassPlugin,
]).process(cssText);
result = ast.css;
} catch (error) {
console.warn('Failed to adapt css for replay', error);
}

cache?.stylesWithHoverClass.set(cssText, result);
return result;
}
Expand Down Expand Up @@ -102,15 +108,44 @@ export function applyCssSplits(
// unexpected: remerge the last two so that we don't discard any css
cssTextSplits.splice(-2, 2, cssTextSplits.slice(-2).join(''));
}
let adaptedCss = '';
if (hackCss) {
adaptedCss = adaptCssForReplay(cssTextSplits.join(''), cache);
}
let startIndex = 0;
for (let i = 0; i < childTextNodes.length; i++) {
if (i === cssTextSplits.length) {
break;
}
const childTextNode = childTextNodes[i];
const cssTextSection = cssTextSplits[i];
if (childTextNode && cssTextSection) {
// id will be assigned when these child nodes are
// iterated over in buildNodeWithSN
childTextNode.textContent = hackCss
? adaptCssForReplay(cssTextSection, cache)
: cssTextSection;
if (!hackCss) {
childTextNode.textContent = cssTextSplits[i];
} else if (i < cssTextSplits.length - 1) {
let endIndex = startIndex;
let endSearch = cssTextSplits[i + 1].length;

// don't do hundreds of searches, in case a mismatch
// is caused close to start of string
endSearch = Math.min(endSearch, 30);

let found = false;
for (; endSearch > 2; endSearch--) {
const searchBit = cssTextSplits[i + 1].substring(0, endSearch);
const searchIndex = adaptedCss.substring(startIndex).indexOf(searchBit);
found = searchIndex !== -1;
if (found) {
endIndex += searchIndex;
break;
}
}
if (!found) {
// something went wrong, put a similar sized chunk in the right place
endIndex += cssTextSplits[i].length;
}
childTextNode.textContent = adaptedCss.substring(startIndex, endIndex);
startIndex = endIndex;
} else {
childTextNode.textContent = adaptedCss.substring(startIndex);
}
}
}
Expand Down
77 changes: 62 additions & 15 deletions packages/rrweb-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,19 @@ export function absolutifyURLs(cssText: string | null, href: string): string {
* Intention is to normalize by remove spaces, semicolons and CSS comments
* so that we can compare css as authored vs. output of stringifyStylesheet
*/
export function normalizeCssString(cssText: string): string {
return cssText.replace(/(\/\*[^*]*\*\/)|[\s;]/g, '');
export function normalizeCssString(
cssText: string,
/**
* _testNoPxNorm: only used as part of the 'substring matching going from many to none'
* test case so that it will trigger a failure if the conditions that let to the creation of that test arise again
*/
_testNoPxNorm = false,
): string {
if (_testNoPxNorm) {
return cssText.replace(/(\/\*[^*]*\*\/)|[\s;]/g, '');
} else {
return cssText.replace(/(\/\*[^*]*\*\/)|[\s;]/g, '').replace(/0px/g, '0');
}
}

/**
Expand All @@ -463,19 +474,24 @@ export function normalizeCssString(cssText: string): string {
export function splitCssText(
cssText: string,
style: HTMLStyleElement,
_testNoPxNorm = false,
): string[] {
const childNodes = Array.from(style.childNodes);
const splits: string[] = [];
let iterLimit = 0;
let iterCount = 0;
if (childNodes.length > 1 && cssText && typeof cssText === 'string') {
let cssTextNorm = normalizeCssString(cssText);
let cssTextNorm = normalizeCssString(cssText, _testNoPxNorm);
const normFactor = cssTextNorm.length / cssText.length;
for (let i = 1; i < childNodes.length; i++) {
if (
childNodes[i].textContent &&
typeof childNodes[i].textContent === 'string'
) {
const textContentNorm = normalizeCssString(childNodes[i].textContent!);
const textContentNorm = normalizeCssString(
childNodes[i].textContent!,

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

View workflow job for this annotation

GitHub Actions / ESLint Check and Report Upload

Forbidden non-null assertion
_testNoPxNorm,
);
const jLimit = 100; // how many iterations for the first part of searching
let j = 3;
for (; j < textContentNorm.length; j++) {
if (
Expand All @@ -489,31 +505,62 @@ export function splitCssText(
break;
}
for (; j < textContentNorm.length; j++) {
const bit = textContentNorm.substring(0, j);
let startSubstring = textContentNorm.substring(0, j);
// this substring should appears only once in overall text too
const bits = cssTextNorm.split(bit);
let cssNormSplits = cssTextNorm.split(startSubstring);
let splitNorm = -1;
if (bits.length === 2) {
splitNorm = cssTextNorm.indexOf(bit);
if (cssNormSplits.length === 2) {
splitNorm = cssNormSplits[0].length;
} else if (
bits.length > 2 &&
bits[0] === '' &&
cssNormSplits.length > 2 &&
cssNormSplits[0] === '' &&
childNodes[i - 1].textContent !== ''
) {
// this childNode has same starting content as previous
splitNorm = cssTextNorm.indexOf(bit, 1);
splitNorm = cssTextNorm.indexOf(startSubstring, 1);
} else if (cssNormSplits.length === 1) {
// try to roll back to get multiple matches again
startSubstring = startSubstring.substring(
0,
startSubstring.length - 1,
);
cssNormSplits = cssTextNorm.split(startSubstring);
if (cssNormSplits.length <= 1) {
// no split possible
splits.push(cssText);
return splits;
}
j = jLimit + 1; // trigger end of search
} else if (j === textContentNorm.length - 1) {
// we're about to end loop without a split point
splitNorm = cssTextNorm.indexOf(startSubstring);
}
if (cssNormSplits.length >= 2 && j > jLimit) {
const prevTextContent = childNodes[i - 1].textContent;
if (prevTextContent && typeof prevTextContent === 'string') {
// pick the first matching point which respects the previous chunk's approx size
const prevMinLength = normalizeCssString(prevTextContent).length;
splitNorm = cssTextNorm.indexOf(startSubstring, prevMinLength);
}
if (splitNorm === -1) {
// fall back to pick the first matching point of many
splitNorm = cssNormSplits[0].length;
}
}
if (splitNorm !== -1) {
// find the split point in the original text
let k = Math.floor(splitNorm / normFactor);
for (; k > 0 && k < cssText.length; ) {
iterLimit += 1;
if (iterLimit > 50 * childNodes.length) {
iterCount += 1;
if (iterCount > 50 * childNodes.length) {
// quit for performance purposes
splits.push(cssText);
return splits;
}
const normPart = normalizeCssString(cssText.substring(0, k));
const normPart = normalizeCssString(
cssText.substring(0, k),
_testNoPxNorm,
);
if (normPart.length === splitNorm) {
splits.push(cssText.substring(0, k));
cssText = cssText.substring(k);
Expand Down
Loading

0 comments on commit 4309de7

Please sign in to comment.