Update to React 17.0.1 #26847
Closed
Update to React 17.0.1 #26847
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
the
Framework
youknowriad
requested review from
ajitbohra,
spacedmonkey and
TimothyBJacobs
as code owners
|
Size Change: -354 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
update/react-17
branch
from
777e9ac to
5f832cc
Compare
hc0503
approved these changes
Nov 10, 2020
|
On the React Native front, @ceyhun is working on an RN upgrade to RN team upgraded React to |
That's right. This is main the PR: wordpress-mobile/gutenberg-mobile#2580
Seems like it'll be released as part of 0.64. Its status can be followed here: react-native-community/releases#207 |
|
@ceyhun, thanks for commenting, by the status issue you shared:
in the comments I've found react-native-community/releases#207 (comment) posted 4 days ago:
Do you think it would be possible to change this in-progress PR to upgrade to RN 0.64 once it's published or it's rather something that needs to happen separately? It looks like those version upgrades are very time-consuming. |
|
When it's out I will give it a try in another branch and see if it breaks something. |
|
@ceyhun Looks like it's out. I'm going to take a look at the test failures on this PR. It would be good to check the mobile too. |
|
I only see 0.64.0-rc.1: |
|
mmm, you're right, I saw a tweet which mislead me. it was probably talking about the RC. |
youknowriad
force-pushed
the
update/react-17
branch
from
5f832cc to
ba3d599
Compare
youknowriad
force-pushed
the
update/react-17
branch
from
ba3d599 to
00a27f2
Compare
gziolo
reviewed
Feb 9, 2021
| @@ -31,10 +31,10 @@ | |||
| ], | |||
| "main": "index.js", | |||
| "dependencies": { | |||
| "@wojtekmaj/enzyme-adapter-react-17": "^0.3.1", | |||
There was a problem hiding this comment.
So it looks like Airbnb no longer maintains enzyme ...
There was a problem hiding this comment.
It's community maintained, but the PR for React 17 support is still WIP: enzymejs/enzyme#2430
There was a problem hiding this comment.
Well, apparently the unofficial adapter we're using has an ongoing issue with unmount not working as expected, so that's why the <ScrollLock> test is failing: wojtekmaj/enzyme-adapter-react-17#12. I'll check for progress there. Official PR is still WIP.
There was a problem hiding this comment.
FYI, this test should now work as the bugifx was just merged & released. Please update and check!
youknowriad
force-pushed
the
update/react-17
branch
from
270f0ee to
7855796
Compare
|
https://github.com/facebook/react-native/releases/tag/v0.64.0 with React 17 is out 🎉 |
|
There is one failing e2e test that looks like an actual issue:
Maybe this branch only needs another rebase. |
gwwar
reviewed
Mar 23, 2021
There was a problem hiding this comment.
Some inserter testing on Safari:
Safari does have some bad inserter behavior :
Navigation Link (Safari):
safarinavlink.mp4
Navigation Link Expected Behavior(Chrome):
chromenavlink.mp4
Columns fail to add items (Safari)
columnssafari.mp4
Random WSOD I ran into
This one might be unrelated/harder to reproduce. But might be worth bulletproofing onCopy in the ClipboardButton. I'm seeing an editor WSOD sometimes when we select all blocks, then delete:
youknowriad
requested review from
noisysocks,
oandregal,
talldan and
tellthemachines
as code owners
|
Okay, snapshots were safe to update, was related to this change in reakit. This hopefully concludes it. Hope I didn't mess anything up rebasing... |
vdwijngaert
force-pushed
the
update/react-17
branch
from
1315a7c to
dbb4b3d
Compare
|
"All" that's left to do now is... fix e2e tests and apparently upgrade to RN 0.64. |
7 tasks
|
Hello again 👋 We're almost done with the React Native 0.64 upgrade on #29118. What do you think about closing this PR after the |
|
I think we don't have to close this PR, just changing the base branch should do the job? Ideally, if both PRs are pretty big, then we can merge those into a shared branch, and merge that branch to trunk instead. So that reviewing and reverting would be easier. |
|
You mean to change the base branch of this PR to the RN 0.64 upgrade branch and target merging RN 0.64 upgrade branch directly to I think there's already some overlap with changes between the two and this PR seems like a smaller change compared to RN 0.64 upgrade. If we have a shared branch it could be tedious to keep up with |
youknowriad
force-pushed
the
update/react-17
branch
from
dbb4b3d to
9d69bd7
Compare
youknowriad
added
the
Needs Dev Note
13 tasks
|
@youknowriad, I think we can close it now that all the changes were included in #29118 🎉 |
youknowriad
removed
the
Needs Dev Note
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #24485.
Like see on the React release post https://reactjs.org/blog/2020/08/10/react-v17-rc.html and as discussed with #core-js it should be okey to upgrade React on Core to 17 even if it's a major version because of the breaking changes are very small. A dev note linking to the React upgrade documentation will be needed.
That said, I opened this PR to just make some checks and see if the tests are ok. We can't merge it until react-native drops a version supporting React 17.
TODO