-
-
Notifications
You must be signed in to change notification settings - Fork 399
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: better sheet and dynamic rules management, leverage useInsertionEffect and support React 18 #1604
Conversation
Looking good, gonna have a deeper look soon, in the meantime anybody could try this on a large codebase and let me know if it breaks anything? |
I've recently deployed this patch to a very complex and big React 18 enabled app, leveraging all benefits of modern React - it works flawlessly as of time of writing. Sadly, I am not deep into react-jss to prove the fix with tests, but I hope others upgrading React will find this PR and help nurture it. 😄 |
This is evtl not testable with tests without going into internals, which isn't a great idea |
Now using the new |
nice! |
Will the request be accepted soon? |
Hyped for this 👏 Nice work! Hopefully this passes review. |
Hey @kof, sorry to bother. Is this PR still on your todo-list perhaps? Thanks a bunch for developing an amazing lib! |
Sorry for the delay, will release it today |
I was a bit too optimistic, just ran the tests locally and some are failing, can you fix them please? Feel free to send a separate PR |
I ran the tests too and got a bunch of Maybe the problem is on my side, can you share the specific tests failing or whether I am doing something wrong? |
My bad, had to |
All tests pass #1608. |
When will the new version be released? |
released as v10.9.1-alpha.1 |
Corresponding Issue(s):
Closes #1597, starting point was #1598
What Would You Like to Add/Fix?
React 18 StrictMode runs each
useEffect
twice. This is done for a good reason where components experience this behaviour in production environments regularly.Without this fix, the UI will break where components rerender fast. (See examples in #1597).
This fix makes sure the dynamic styles are loaded after the sheet was managed.
Todo