Upgrade to RN 0.72.12 #1161
Merged
Upgrade to RN 0.72.12 #1161
Conversation
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
7553e4d to
9c0f6b4
Compare
9c0f6b4 to
96b1daa
Compare
82195be to
186917a
Compare
186917a to
dfce6dc
Compare
Automatically added during pod install.
We need to install React 18 even if it may not be used. See https://reactnative.dev/docs/react-18-and-react-native
I had to create a "release" folder to add the new empty ReactNativeFlipper.java. It is not possible to put it in our product flavor "main" folder because in this case gradle complains about duplicate classes. See facebook/react-native#36362 (comment)
"In new react-native 0.71.* have been change implementation of AppDelegate.mm to simplify future upgrade process, that hide access to rootView under the hood." We need to get the "rootView" from self.window.rootViewController.view according to this issue which became the official solution : zoontek/react-native-bootsplash#421
addEventListener() AppState.removeEventListener has been removed in RN0.71.
As stated in the README "Using react-native >= 0.71? install react-native-restart@0.0.27 and above", so let's update.
React Native <= 0.70 was polyfilling global.Promise. They removed it (which is good), but it breaks some of our tests. So let's add it again for the moment so we can finish this upgrade. https://github.com/facebook/react-native/pull/34659/files#diff-759a02c8fa99d555891bb2a2d3ae2495a2295dc1e1183dc8ba6281b0e172208a
@testing-library/react-hooks does not support React 18. renderHook has been moved to @testing-library/react and @testing-library/react-native. Migrated tests using it.
dfce6dc to
c736efb
Compare
zatteo
commented
Jun 27, 2024
| @@ -1010,14 +1010,15 @@ | |||
| COPY_PHASE_STRIP = NO; | |||
| ENABLE_STRICT_OBJC_MSGSEND = YES; | |||
| ENABLE_TESTABILITY = YES; | |||
| "EXCLUDED_ARCHS[sdk=iphonesimulator*]" = "arm64 i386"; | |||
| "EXCLUDED_ARCHS[sdk=iphonesimulator*]" = i386; | |||
There was a problem hiding this comment.
I don't remember what are the consequences of this ?
Ldoppea
reviewed
Jun 28, 2024
| android { | ||
| ndkVersion rootProject.ext.ndkVersion | ||
|
|
||
| compileSdkVersion rootProject.ext.compileSdkVersion | ||
|
|
||
| namespace "io.cozy.flagship.mobile" |
There was a problem hiding this comment.
With namespace declared here, should we remove the one on defaultConfig in line 100?
There was a problem hiding this comment.
I kept this one to be as close as default React Native configuration.
Tested building and running both brand app successfully.
| @@ -46,10 +45,10 @@ | |||
| <activity | |||
| android:name=".MainActivity" | |||
| android:label="@string/app_name" | |||
| android:configChanges="keyboard|keyboardHidden|orientation|screenSize|uiMode" | |||
| android:configChanges="keyboard|keyboardHidden|orientation|screenLayout|screenSize|smallestScreenSize|uiMode" | |||
There was a problem hiding this comment.
Maybe we should check how our app behaves on split screens as it seems to be the default configuration now
Cf: https://developer.android.com/guide/topics/large-screens/multi-window-support
There was a problem hiding this comment.
Looks good. Tested logging in, navigating between apps and some apps (Drive, Notes, Settings, CoachCO2, mespapiers).
We keep the namespace from React Native default configuration.
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.
Prerequisites
Android
✅ Build
✅ Start
iOS
✅ Build
✅ Start
Todo (dev)
Merge upstream of https://github.com/cozy/react-native-bootsplash to at least 4.X.X (but 5.X.X is already here and stable)(see 12830f1)AppState.removeEventListenercall by.remove()on the listener returned byAppState.addEventListener()https://github.com/cozy/cozy-flagship-app/blob/play-with-rn-upgrade/src/libs/httpserver/HttpServer.ts#L97Todo (test)
Extensive tests on real devices
On a real Android
biometryOn a real iPhone