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

Improve CSS-in-js support for recordings #18520

Closed
4 tasks
pauldambra opened this issue Nov 9, 2023 · 5 comments
Closed
4 tasks

Improve CSS-in-js support for recordings #18520

pauldambra opened this issue Nov 9, 2023 · 5 comments
Labels
bug Something isn't working right feature/replay Features for Team Replay

Comments

@pauldambra
Copy link
Member

pauldambra commented Nov 9, 2023

Bug description

We've recently had a few reports of improper rendering on playback and in both cases it was css-in-js not being inlined and then the sources either having changed between recording and playback or not being available to us at playback (e.g. cors issues loading JS assets)

see

TODO

  • what support does rrweb have already
    • are we init-ing incorrectly
  • test reverse proxy with JS to get around CORS issues
  • should the reverse proxy be caching assets as well
@daibhin
Copy link
Contributor

daibhin commented Apr 2, 2024

Internal support request: https://posthoghelp.zendesk.com/agent/tickets/11285

Digging into what is going on here and making notes of everything I discover to aid future investigations...

Styles exist and are being applied on the customers DOM elements:
Screenshot 2024-04-02 at 11 22 53

But the referenced stylesheet is empty:
Screenshot 2024-04-02 at 11 21 49

How can that be? Well it can be. Looks like calling insertRule will have that effect.

And confirmed in the console:
Screenshot 2024-04-02 at 11 24 45
Styles exist but not in the dom.

This means that during playback of recordings the necessary CSS has not been captured which is what causes the incorrect playback issues for CSS-in-JS.


Seems like there is some old RRWeb issues related to insertRule:

rrweb-io/rrweb#563
rrweb-io/rrweb#104

And an issue specifically related to emotion:
emotion-js/emotion#1248

There is an open issue with the exact same problem as reported by this customer but with no progress / resolution:

rrweb-io/rrweb#1230


On the emotion side...

Some suggest clashing keys could be causing them issues:
https://stackoverflow.com/questions/69980394/emotion-cache-on-ssr-with-material-ui-is-always-empty

@daibhin
Copy link
Contributor

daibhin commented Apr 9, 2024

Ok, a ton more investigations into this...

I'm back to thinking this is a capture bug. I've primarily been following a missing class on the button. What was confusing is that the class seemed to exist in the snapshot data... but I've noticed that it is only first included in the next full snapshot.

This suggests that the CSS rule is inserted when the page navigates but RRWeb doesn't capture the insertion. Instead the class only becomes available once a follow up full snapshot is taken (at which point the user might have navigated away from the page in question making it look like the styles never existed)

@daibhin
Copy link
Contributor

daibhin commented Apr 25, 2024

Coming back to this as we understand a lot more about how CSS is handled in RRWeb. CSS in JS is supported as the styles should be inlined and reapplied in a style tag.

Recent investigations found that the ast parser included in RRWeb is ported from https://github.com/reworkcss/css. On top of that regex expressions are used to mutate the CSS for two purposes:

  1. Convert pseudo hover selectors to classes (so hover styles can be shown during playback)
  2. To replace {min|max}-device-{width|height} with $1-$2 e.g. min-device-width to min-width

Both of these could be better achieved using AST mutations over regex expressions. The original implementation dates back to 2018

I am attempting to do this in rrweb-io/rrweb#1458

PostCSS seems like the best choice when considering benchmarks and the fact that it is well maintained with commits in the last month versus Rework CSS (which is what is currently used, contains the selector bugs and hasn't had updates in 3+ years) or CSSOM which actively says it is no longer maintained.

(I originally tried CSSTree which seemed promising but adapting the AST was difficult and would probably still rely on the regex expressions)

Getting rrweb-io/rrweb#1458 over the line might take a while so in the meantime we could:

  1. Add an RRWeb plugin that preprocesses the _cssText using postcss ourselves
  2. Patch RRWeb to set skip the mutation internally in the package

@daibhin
Copy link
Contributor

daibhin commented Apr 30, 2024

rrweb-io/rrweb#1458 is now ready to go and passing all tests (locally at least). Might take some time to get it over the line so I'm going to move ahead with adapting the changes and applying them directly in PostHog - hoping this will fix a lot of our broken CSS issues

@daibhin
Copy link
Contributor

daibhin commented Jun 17, 2024

Update: We're made as much progress on CSS as possible. Our last remaining known issues are around shorthand variables not being correctly parsed during capture - rrweb-io/rrweb#1322.

I think the final improvements for customers will come once rrweb-io/rrweb#1475 lands. We should hold off working on issues likely to be fixed in favour of helping get that PR over the line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right feature/replay Features for Team Replay
Development

No branches or pull requests

3 participants