Upgrade to React 18 #32765
Upgrade to React 18 #32765
Conversation
youknowriad
requested review from
ajitbohra,
spacedmonkey and
TimothyBJacobs
as code owners
|
Size Change: +4.55 kB (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
gziolo
added
[Package] Element
|
Note that StrictMode is "stricter" in 18 (details). So if parts of your tree are broken after switching to |
|
Thanks for chiming in @gaearon and for the hint. My understanding was that The good news is that React 18 is definitely faster than React 17 without any change. (according to our perf metrics)
|
Yes. Just pointing out in case you run into this later. |
youknowriad
force-pushed
the
try/react-18
branch
from
f850c62 to
233d1f7
Compare
youknowriad
force-pushed
the
try/react-18
branch
from
233d1f7 to
a1bd241
Compare
|
Time to resume testing - https://reactjs.org/blog/2022/03/29/react-v18.html |
|
Indeed, I'm curious about react-native timing though which would be a requirement for us before upgrading. |
youknowriad
force-pushed
the
try/react-18
branch
from
a1bd241 to
3b03136
Compare
|
Updated to the stable version, I'm working on fixing some unit tests. But given the number of failures in our CI, this is probably going to require a group effort. |
|
It looks like most tests that fail use |
That would be great to finally make that happen. I'm all in favor of getting rid of It looks like similar issues apply to the testing tools from the React team. @youknowriad, can you share some details what you discovered while testing some of the tests to help others? |
|
Yes, so here are the few things I discovered so far: both I considered silencing these by accepting that these warnings are thrown (I don't know how to do it though), but then I saw that there was only like 30 tests remaining. I thought it could be doable to update these but I'm not sure :P |
|
Anyway, I'm taking a break from this PR for now, please if you want to help with the tests, go for it :P |
|
The other thing is that |
youknowriad
force-pushed
the
try/react-18
branch
2 times, most recently
from
1e9f4e3 to
2124747
Compare
| @@ -259,7 +259,7 @@ describe( 'UnitControl', () => { | |||
| ); | |||
| } ); | |||
|
|
|||
| it( 'should invoke onChange and onUnitChange callbacks when isPressEnterToChange is true and the component is blurred with an uncommitted value', async () => { | |||
| it.skip( 'should invoke onChange and onUnitChange callbacks when isPressEnterToChange is true and the component is blurred with an uncommitted value', async () => { | |||
There was a problem hiding this comment.
This is potentially a breaking change somehow or maybe we need to check something internally but basically, there's a callback being called twice now.
youknowriad
force-pushed
the
try/react-18
branch
from
2124747 to
9c504f7
Compare
|
I fixed all the unit tests, I think there's one that might be a real failure due to React 18. I skipped it for now until I investigate it more deeply. #32765 (comment) I'd like to look at e2e tests later. |
youknowriad
force-pushed
the
try/react-18
branch
from
9c504f7 to
7eb8d59
Compare
|
React Native 0.69 was released a few days ago - https://reactnative.dev/blog/2022/06/21/version-069. |
|
Hey there 👋 we are working on the React native upgrade that uses React 18. I was wondering who's working on this PR as we will need to pull these changes, there are quite a few conflicts at the moment 😅 |
|
Hi there! no one is working on this PR at the moment. I just got back from a long break, so I might not be able to pick it up right away. Please feel free to take over for the moment. |
|
Noting that I've actively been working on removing |
|
Any news on this? |
|
We're hoping to reattempt it the following days, now that Enzyme is gone. |
|
Hey, I was trying to see whether this upgrade would upgrade |
|
Having looked through all the changes in this PR, here is a summary of what I saw and what could be a way forward: Unit tests: Many removals of The only part of unit tests changes that's not in Types: Several files are modified only to fix type annotations ( Introducing Event timing in e2e tests: There are many places, e.g., the Component bugfixes: Here React 18 apparently uncovered some existing bugs:
And that's all, the remaining changes are straightforward: add |
|
Thanks for the summary, @jsnajdr, that's really helpful! I've addressed a few of those legacy rendering ones in the following PRs:
I'm planning to be working on the rest soon. |
I think the previous implementation was broken in the sense of that it relied on React lifecycle too much (I knew the exact reason but forgot :) ) which caused sometimes the preview page to not be set properly. The refactor I did was to actually move the logic to an action creator instead of being split across lifecycle methods (if I remember properly) |
|
Opened a few more instances to tackle regarding changes in tests - we're using |
I created a PR here: #44680 |
The code that does save/autosave and then, in a lifecycle, watches the previewWindow = createWindow();
writeInterstitial( previewWindow );
if ( needsSaving ) {
await save();
}
previewWindow.document.location = previewLink;should be much more reliable. |
|
I think we can close this one in favor of #45235 which is pretty close to the finish line, cc @youknowriad. In any case, this one is well behind |
|
Yep. Thanks for your work on this @tyxla |
Closes #41559.
Fixes #43077.
Do not merge.
React 18 is still in alpha version and might not be released before months, but it's always good to check impact and give feedback.
It also comes with a nice set of concurrency features that could prove to be good for performance reasons.
This PR just tries to upgrade to the alpha version to check the tests and all.