facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.89k stars 46.52k forks source link

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

Open souporserious opened 1 month ago

souporserious commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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:

https://github.com/user-attachments/assets/036eb435-4324-4190-a892-c1a4820e0e9a

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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