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

[React 19] style using precedence can produce many additional style elements after initial render #30739

Open
souporserious opened this issue Aug 17, 2024 · 6 comments

Comments

@souporserious
Copy link

Summary

When adding multiple style elements with the same precedence they will only ever be grouped on the initial render. During subsequent style elements being discovered they will each attach a new style element in the head of the document which can result in many elements being inserted as seen below:

image

I'm not sure if there is an inherent constraint because of concurrent rendering, but it would be nice if React could either batch these similar to the initial page render or always reuse a precedence once created and insert rules into the same style tag.

@eps1lon
Copy link
Collaborator

eps1lon commented Aug 19, 2024

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://react.new), or a link to a repository on GitHub.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

@souporserious
Copy link
Author

Here's a repro using a Next.js Codesandbox:
https://codesandbox.io/p/devbox/multiple-style-elements-dhhq2t

The two style elements initially rendered are grouped under one style tag, however, after clicking the button to add additional style elements they will continue to render more style elements and not group them under the same precedence.

I'm currently using this to build an atomic CSS in JS library which the original screenshot above is from. It seems React could reuse the existing precedence style element if it already exists and insert new rules there.

@eps1lon
Copy link
Collaborator

eps1lon commented Aug 19, 2024

however, after clicking the button to add additional style elements they will continue to render more style elements and not group them under the same precedence.

I can't reproduce this. The new style elements are added to the previous precedence group:

Screen.Recording.2024-08-19.at.21.29.01.mov

But it's also not really a meaningful tests because there's only stylesheets with a single precedence.

precedence is not meant to merge stylesheets. They're simply grouped together so that e.g. in

<style precedence="first" />
<style precedence="second" />

rendering another <style precedence="first" /> would result in

<style precedence="first" />
<style precedence="first" />
<style precedence="second" />

@souporserious
Copy link
Author

souporserious commented Aug 19, 2024

But it's also not really a meaningful tests because there's only stylesheets with a single precedence.

Sorry, me using the term grouping was confusing here. It groups them correctly, but this is what I wanted to showcase, that using a single precedence can still result in many elements being appended to the head.

precedence is not meant to merge stylesheets.

This was the issue I was hoping could be optimized. Is there a reason styles under the same precedence can't be merged into the same stylesheet like they are on initial page load?

@eps1lon
Copy link
Collaborator

eps1lon commented Aug 19, 2024

Is there a reason styles under the same precedence can't be merged into the same stylesheet like they are on initial page load?

It would be harder to reconcile if some of the elements are removed later or their contents changed. We'd first have to only merge stylesheets with the exact same attributes and then insert markers so that we can safely remove contents later defeating any potentially byte-savings merging would have.

Are there other reasons why you want <style /> tags merged?

@souporserious
Copy link
Author

It would be harder to reconcile if some of the elements are removed later or their contents changed.

This part is confusing to me since it already does this on initial render, so React must add some sort of marker like this already?

I've also noticed in my use case that React will never get rid of the style once it's been created which the docs do mention it might be removed, but it is opaque to understand how to work around right now as a library author since I'm currently making the assumption once it's created it will never be removed.

Are there other reasons why you want <style /> tags merged?

No other reason, just didn't want to bloat the document head if possible. Feel free to close this issue if it is an inherent constraint of how style hoisting works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants