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

replay :hover rewrite - instead mutate stylesheets #1480

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

eoghanmurray
Copy link
Contributor

@eoghanmurray eoghanmurray commented May 17, 2024

switch to model where we adapt the replay stylesheets programmatically after they have been added to the page instead of parsing the text

  • means we don't have to rely on regexp parsing of raw cssText text value to ensure we are substituting css selectors and media statements correctly
  • could be adapted in future for use against external (non inlined) <link> stylesheets during replay

#1458 points out failings in the current css parsing, and switches to postcss. This is an alternative to that and removes all 3rd party dependencies.

This PR could also be a contender as a fix for #1478 (although I suspect something else is the matter there, as I couldn't create a stylesheet with the CSS text from that)

How to help:

  • satisfy eslint by removing the ignore line and upgrading typescript to a version that incorporates Add StyleSheetList and CSSRuleList to dom.iterable.d.ts microsoft/TypeScript#23406 bugfix
  • ensure we are applying adaptStylesheetForReplay after a virtual dom operation (i.e. fastforward in replayer which has a diff of adding a new stylesheet)
  • add back the caching support (I'm not sure how as the stylesheet for which we've adapted things is mutable)
  • verify replay performance is better or isn't affected

Copy link

changeset-bot bot commented May 17, 2024

🦋 Changeset detected

Latest commit: 0a04127

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
rrweb-snapshot Patch
rrweb Patch
rrdom Patch
rrdom-nodejs Patch
rrweb-player Patch
@rrweb/types Patch
@rrweb/web-extension Patch
rrvideo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@eoghanmurray eoghanmurray changed the title replay mutate stylesheets replay :hover rewrite - instead mutate stylesheets May 17, 2024
@eoghanmurray eoghanmurray force-pushed the replay-mutate-stylesheets branch from 4177e18 to 17459cd Compare May 20, 2024 11:46
import { createMirror, Mirror, stringifyStylesheet } from '../src/utils';

/*
function toEqualCss(actual: string, expected: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made an attempt to write my own expect matcher, but gave up due to typescript overhead and wrote norm instead

…s programmatically after it's added to the page instead of parsing the text

 - means we don't have to rely on regexp parsing to get css selectors and media statements
 - could be adapted for use against <link> stylesheets during replay
…lesheetForReplay` until the <style> element is in the DOM
…rantee that the adapt process will not rewrite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant