Closed enisdenjo closed 2 years ago
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 useInsertionEffect
, as recommended by the React Team, together with a few more improvements and simplifications.
nice!
Will the request be accepted soon?
Hyped for this 👏 Nice work! Hopefully this passes review.
nice!
Hey @kof, sorry to bother. Is this PR still on your todo-list perhaps? It has been a couple of weeks and I'm saddened that I can't use one of my favourite tools with the latest version of React.
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 Error: Can't resolve
s. I don't think they are caused by this PR, or?
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 yarn build
. Will be inspecting the tests.
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