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

fix(engine-dom): append global stylesheets to document.head #2923

Merged
merged 5 commits into from
Jul 8, 2022

Conversation

nolanlawson
Copy link
Contributor

Details

In #2827 we started using document.adoptedStyleSheets instead of injecting <style>s into the document.head (in supported browsers). Unfortunately this breaks existing code that expects that it can crawl the CSS for all elements on the page by iterating through document.styleSheets. (As it turns out, document.adoptedStyleSheets does not affect document.styleSheets – they are separate collections.)

I opened #2922 to track removing this once it's safe to go back to using document.adoptedStyleSheets.

Does this pull request introduce a breaking change?

  • ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • ⚠️ Yes, it does include an observable change.

Gus WI

W-11393601

@nolanlawson nolanlawson changed the title fix: append global stylesheets to document.head fix(engine-dom): append global stylesheets to document.head Jul 7, 2022
@nolanlawson nolanlawson requested a review from pmdartus July 7, 2022 17:52
@nolanlawson
Copy link
Contributor Author

nolanlawson commented Jul 7, 2022

Surprisingly, this seems to be a slight perf improvement (in Chrome) in synthetic shadow and light DOM when rendering different styles, presumably because, even with mutable adopted stylesheets, it's faster to inject <style>s into the head than to mutate a large adoptedStyleSheets array.

Screen Shot 2022-07-07 at 12 26 32 PM

Color me surprised, but in any case I'm glad there's no regression. (The first benchmark is a 1.5ms difference, so I don't really trust it.)

@nolanlawson nolanlawson merged commit de353e3 into master Jul 8, 2022
@nolanlawson nolanlawson deleted the nolan/document-head-styles branch July 8, 2022 16:17
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