Skip to content

fix: remote CSS does not get rebuilt properly#226

Merged
billyvg merged 1 commit intosentry-v2from
fix-snapshot-remote-css-remove-attributes
Dec 10, 2024
Merged

fix: remote CSS does not get rebuilt properly#226
billyvg merged 1 commit intosentry-v2from
fix-snapshot-remote-css-remove-attributes

Conversation

@billyvg
Copy link
Member

@billyvg billyvg commented Dec 6, 2024

This fixes an issue where inlined CSS from a remotely loaded <link> does not get applied properly due to object reference mutation.

This fixes an issue where inlined CSS from a remotely loaded `<link>` does not get applied properly due to object reference mutation.
cache: this.cache,
},
);
const newNode = buildNodeWithSN(newSn, {
Copy link
Member Author

Choose a reason for hiding this comment

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

If you look above at the Object.assign call, you'll see that we mutate the meta object reference from the mirror and pass it to buildNodeWithSN(). Looking inside of buildNodeWithSN() you can see that it ends up comparing the same object reference and returning the node from the mirror instead of building a new node.

if (mirror.has(n.id)) {
    const nodeInMirror = mirror.getNode(n.id)!;
    const meta = mirror.getMeta(nodeInMirror)!;
    if (isNodeMetaEqual(meta, n)) return mirror.getNode(n.id);
  }

This meant that remote stylesheets were always loaded remotely (even if you inline) and could be blocked by CORS, making the inlined CSS rules unused.

@billyvg billyvg marked this pull request as ready for review December 6, 2024 17:55
@billyvg billyvg requested a review from a team December 6, 2024 17:55
Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

Wild catch!

@billyvg billyvg merged commit ba5a067 into sentry-v2 Dec 10, 2024
@billyvg billyvg deleted the fix-snapshot-remote-css-remove-attributes branch December 10, 2024 18:49
billyvg added a commit to getsentry/sentry-javascript that referenced this pull request Dec 12, 2024
Includes the following fixes:

- fix: remote CSS does not get rebuilt properly ([#226](getsentry/rrweb#226))
- fix(snapshot): Set <link> attributes to null for remote CSS ([#227](getsentry/rrweb#227))
- fix(snapshot): Change to ignore all link[rel="modulepreload"] ([#228](getsentry/rrweb#228))
billyvg added a commit to getsentry/sentry-javascript that referenced this pull request Dec 12, 2024
Includes the following fixes:

- fix: remote CSS does not get rebuilt properly ([#226](getsentry/rrweb#226))
- fix(snapshot): Set <link> attributes to null for remote CSS ([#227](getsentry/rrweb#227))
- fix(snapshot): Change to ignore all link[rel="modulepreload"] ([#228](getsentry/rrweb#228))
billyvg added a commit to getsentry/sentry-javascript that referenced this pull request Dec 12, 2024
Includes the following fixes:

- fix: remote CSS does not get rebuilt properly
([#226](getsentry/rrweb#226))
- fix(snapshot): Set <link> attributes to null for remote CSS
([#227](getsentry/rrweb#227))
- fix(snapshot): Change to ignore all link[rel="modulepreload"]
([#228](getsentry/rrweb#228))
billyvg added a commit that referenced this pull request Dec 17, 2024
This fixes an issue where inlined CSS from a remotely loaded `<link>`
does not get applied properly due to object reference mutation.
billyvg added a commit that referenced this pull request Dec 17, 2024
This fixes an issue where inlined CSS from a remotely loaded `<link>`
does not get applied properly due to object reference mutation.
billyvg added a commit that referenced this pull request Dec 17, 2024
This fixes an issue where inlined CSS from a remotely loaded `<link>`
does not get applied properly due to object reference mutation.
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.

2 participants