Closed
Conversation
|
Size Change: -354 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
777e9ac to
5f832cc
Compare
hc0503
approved these changes
Nov 10, 2020
Member
|
On the React Native front, @ceyhun is working on an RN upgrade to RN team upgraded React to |
Member
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 |
Member
|
@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. |
Member
|
When it's out I will give it a try in another branch and see if it breaks something. |
Contributor
Author
|
@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. |
Member
|
I only see 0.64.0-rc.1: |
Contributor
Author
|
mmm, you're right, I saw a tweet which mislead me. it was probably talking about the RC. |
5f832cc to
ba3d599
Compare
ba3d599 to
00a27f2
Compare
gziolo
reviewed
Feb 9, 2021
| ], | ||
| "main": "index.js", | ||
| "dependencies": { | ||
| "@wojtekmaj/enzyme-adapter-react-17": "^0.3.1", |
Member
There was a problem hiding this comment.
So it looks like Airbnb no longer maintains enzyme ...
Member
There was a problem hiding this comment.
It's community maintained, but the PR for React 17 support is still WIP: enzymejs/enzyme#2430
Member
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!
270f0ee to
7855796
Compare
Member
|
https://github.com/facebook/react-native/releases/tag/v0.64.0 with React 17 is out 🎉 |
Member
|
There is one failing e2e test that looks like an actual issue:
Maybe this branch only needs another rebase. |
gwwar
reviewed
Mar 23, 2021
Contributor
gwwar
left a comment
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:
Contributor
|
Also seeing similar regressions in Firefox: firefoxinserter.mp4 |
Member
|
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... |
1315a7c to
dbb4b3d
Compare
Member
|
"All" that's left to do now is... fix e2e tests and apparently upgrade to RN 0.64. |
7 tasks
Member
|
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 |
Member
|
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. |
Member
|
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 |
dbb4b3d to
9d69bd7
Compare
13 tasks
Member
|
@youknowriad, I think we can close it now that all the changes were included in #29118 🎉 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
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