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

Add support for replaying StyleSheetRule events #178

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

dcramer
Copy link
Contributor

@dcramer dcramer commented Feb 24, 2020

Fixes #58
Fixes #104

@dcramer
Copy link
Contributor Author

dcramer commented Feb 24, 2020

Tested via the snippet below and it seemed to work, though the id recorded was -1 (maybe thats correct?), so I was a bit confused:

https://codesandbox.io/s/beautiful-babbage-hz2wj

{"type":3,"data":{"source":8,"id":-1,"adds":[{"rule":"p{color: red;\n}","index":0}]}

Here's the full output from repl: https://gist.github.com/dcramer/3b8449cf4388663ab7f0c1dbec399fe6

@dcramer dcramer requested a review from Yuyz0112 February 24, 2020 16:57
@dcramer dcramer force-pushed the feat/replay-style-rules branch from c28a90f to 7af5893 Compare February 24, 2020 18:28
@dcramer
Copy link
Contributor Author

dcramer commented Feb 24, 2020

Aside - willing to help try to get test coverage in for replays (beyond just testing the controller actions), but didn't see any existing.

@Yuyz0112
Copy link
Member

Interesting.

The example in the gist works because it captures the added DOM:

{
  "source": 0,
  "texts": [],
  "attributes": [],
  "removes": [],
  "adds": [
    {
      "parentId": 4,
      "previousId": 52,
      "nextId": null,
      "node": {
        "type": 2,
        "tagName": "style",
        "attributes": {
          "_cssText": "p { color: red; }"
        },
        "childNodes": [],
        "id": 53
      }
    }
  ]
}

@Yuyz0112
Copy link
Member

The replayer code looks fine.

I believe the id: -1 happens because of the async callback of MutationObserver.

Looks like the order of the dynamic stylesheet insertion is:

  1. create a <style> element
  2. call insertRule on the style element

But the order of rrweb's observer callback is:

  1. Capture the insertRule calling, because it triggered in sync.
  2. Capture the add of <style> element, because MutationObserver will fire in the next async loop.

I'm not sure whether this works for all the CSS in JS lib, it seems the first insertion will be replayed by the add of DOM, and the following insertion weill be captured correctly.
But I think put the insertRule observer callback into a macro task(maybe by using setTimeout(, 0)) will help us always get the right order.

@Yuyz0112 Yuyz0112 merged commit 9b7f8d6 into rrweb-io:master Feb 25, 2020
@dcramer
Copy link
Contributor Author

dcramer commented Feb 25, 2020

Seems like it’d be nice to ignore insertRule until the style is on the dom. Thoughts? Not sure how complex that’d be. Seems like it’d save some bandwidth on payload size.

billyvg added a commit to p-mazhnik/rrweb that referenced this pull request May 21, 2024
Ref: rrweb-io#1343

Co-authored-by: Michael Dellanoce <michael.dellanoce@gmail.com>
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.

[Observer] Observer doesn't watch for CSSStyleSheet.insertRule Missing styles in recording
2 participants