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

feat: move style injection from render to useInsetionEffect #3821

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexandernanberg
Copy link
Member

Atm this is just a quick experiment if we can utilize useInsertionEffect instead of injecting styles in render which isn't the best for performance. E.g. following the guidelines described in reactwg/react-18#110

From my limited and quick testing this seems to work fine, is there anything I'm missing and does this sound like a good idea to you? If that is the case I'll clean up the PR a little more, e.g. renaming generateAndInjectStyles to something more fitting

@quantizor
Copy link
Contributor

I'll have to look at this one later. I switched us to use the new effect type for global styles but I'm not sure about doing it for component styles since they need to be injected as fast as possible on first render

@alexandernanberg
Copy link
Member Author

One possible solution could be to inject styles in useInsertionEffect if React 18, otherwise fallback to doing it in render like before

@agriffis
Copy link
Contributor

@probablyup

they need to be injected as fast as possible on first render

I haven't looked at the code in React to verify, but the doc claims "it fires synchronously before all DOM mutations."

That seems to satisfy "as fast as possible" but are there situations where this still isn't enough?

Otherwise this LGTM, pending whatever cleanup @alexandernanberg wanted to do.

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.

None yet

3 participants