Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(upgrade): RN v74+ resolution #7886

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sayurimizuguchi
Copy link

@sayurimizuguchi sayurimizuguchi commented Jun 13, 2024

Support V74 React Native.

Environment of a mobile app with 7.40.0:

React Native Navigation version: 7.40.0
React Native version: 0.74.1-0.74.4
Has Fabric (React Native's new rendering system) enabled: NO
Node version: 20.12.2
Device model:
Android SDK: 34

Errors Before changes:

  • || ((oldStyle & Typeface.BOLD) != 0 && weight == ReactTextShadowNode.UNSET)) { ^ symbol: variable UNSET location
  • minSdk ✅
  • babel ✅

After changes:

Build succeeded with React Native V74+ ✅

@sayurimizuguchi sayurimizuguchi marked this pull request as draft June 13, 2024 18:55
@simonecorsato
Copy link

Hi, any chance to get this PR merged and fix the issue?
Thanks

@sayurimizuguchi
Copy link
Author

@d4vidi Let me know if any other changes are needed.

@pedroicandido
Copy link

Hey guys, any news about this pr?

@d4vidi
Copy link
Collaborator

d4vidi commented Jul 24, 2024

@igorgn could we replace this with a buildkite "gateway" approval button so we could run this upon confirmation?
image

@d4vidi
Copy link
Collaborator

d4vidi commented Jul 24, 2024

In any case, thanks @sayurimizuguchi, we will put more attention into this ASAP.
An official upgrade to RN74 is in our plans but we'd be thrilled to get help with that and with that speed things up. There's a lot on our plate.

@sayurimizuguchi
Copy link
Author

sayurimizuguchi commented Jul 24, 2024

In any case, thanks @sayurimizuguchi, we will put more attention into this ASAP. An official upgrade to RN74 is in our plans but we'd be thrilled to get help with that and with that speed things up. There's a lot on our plate.

Let me know if you need extra eyes on the upgrade. I would be glad to help.

@d4vidi d4vidi self-assigned this Jul 25, 2024
@d4vidi
Copy link
Collaborator

d4vidi commented Aug 4, 2024

@sayurimizuguchi I've tried to add you as a friend on Discord; Please approve.
In any case, we're working with Security in order to get this thing going, stay tuned.

@sayurimizuguchi
Copy link
Author

@sayurimizuguchi I've tried to add you as a friend on Discord; Please approve. In any case, we're working with Security in order to get this thing going, stay tuned.

I just accepted. Let me know how it goes; we can continue this discussion on Discord.

@bahtiyarerden
Copy link

Hello guys!

Any chance you can prioritize this PR? RNN doesn't work in RN version 0.74.

@d4vidi
Copy link
Collaborator

d4vidi commented Aug 12, 2024

Hello guys!

Any chance you can prioritize this PR? RNN doesn't work in RN version 0.74.

Yep we're had a few CI issues we had to fix first. Reiterating back to it.

@adammcarth
Copy link

adammcarth commented Aug 12, 2024

@d4vidi What's the latest known React Native version that RNN works with? Is it just 0.74.x that is currently effected?

@bahtiyarerden
Copy link

bahtiyarerden commented Aug 13, 2024

@d4vidi What's the latest known React Native version that RNN works with? Is it just 0.74.x that is currently effected?

@adammcarth I've tested on RN 0.73.9 and working fine.

@d4vidi
Copy link
Collaborator

d4vidi commented Aug 14, 2024

Yep RN 73 is well supported and tested regularly.

@d4vidi
Copy link
Collaborator

d4vidi commented Aug 14, 2024

@sayurimizuguchi it seems like we've got the ball rolling, finally 👌🏻
You can review the CI errors and continue your work over this.

@sayurimizuguchi
Copy link
Author

@sayurimizuguchi it seems like we've got the ball rolling, finally 👌🏻 You can review the CI errors and continue your work over this.

Sounds good. I will be working on the CI logs next

@d4vidi
Copy link
Collaborator

d4vidi commented Aug 22, 2024

Hey @sayurimizuguchi, how are things going? Do you need help making further progress here? We're also largely available on Discord, still

@sayurimizuguchi
Copy link
Author

Hey @sayurimizuguchi, how are things going? Do you need help making further progress here? We're also largely available on Discord, still

I'm sorry for not getting back to you sooner (I work full-time). I added the required resolution to fix the pipeline issues, and the branch was successfully built. I also tested the playground app.

@sayurimizuguchi sayurimizuguchi changed the title Issue ReactTextShadowNode not on v74 resolution feat(upgrade): RN v74.2 resolution Aug 22, 2024
@sayurimizuguchi sayurimizuguchi changed the title feat(upgrade): RN v74.2 resolution feat(upgrade): RN v74+ resolution Aug 22, 2024
@d4vidi
Copy link
Collaborator

d4vidi commented Aug 27, 2024

@sayurimizuguchi Sorry, I've been mostly away in the last couple of days.
I've allowed all the checks to run, expect the real feedback soon enough (green, I hope 🤞🏻)

@d4vidi
Copy link
Collaborator

d4vidi commented Aug 27, 2024

Red, I'm afraid. Seems there's some back-compat things to take care of, but it is a start.

Android:


> Task :react-native-navigation:compileReactNative71ReleaseUnitTestJavaWithJavac
--
  | /Users/builder/uibuilder/work/lib/android/app/src/test/java/com/reactnativenavigation/mocks/TestReactView.java:22: error: method does not override or implement a method from a supertype
  | @Override


iOS:


> [email protected] pod-install /Users/builder/uibuilder/work
--
  | > cd playground/ios && pod install
  |  
  |  
  | [!] Invalid `Podfile` file: uninitialized constant Pod::Podfile::FlipperConfiguration.
  |  
  | #  from /Users/builder/uibuilder/work/playground/ios/Podfile:10
  | #  -------------------------------------------
  | #
  | >  flipper_config = ENV['NO_FLIPPER'] == "1" ? FlipperConfiguration.disabled : FlipperConfiguration.enabled
  | #
  | #  -------------------------------------------


@sayurimizuguchi
Copy link
Author

sayurimizuguchi commented Aug 27, 2024

Red, I'm afraid. Seems there's some back-compat things to take care of, but it is a start.

Android:


> Task :react-native-navigation:compileReactNative71ReleaseUnitTestJavaWithJavac
--
  | /Users/builder/uibuilder/work/lib/android/app/src/test/java/com/reactnativenavigation/mocks/TestReactView.java:22: error: method does not override or implement a method from a supertype
  | @Override

iOS:


> [email protected] pod-install /Users/builder/uibuilder/work
--
  | > cd playground/ios && pod install
  |  
  |  
  | [!] Invalid `Podfile` file: uninitialized constant Pod::Podfile::FlipperConfiguration.
  |  
  | #  from /Users/builder/uibuilder/work/playground/ios/Podfile:10
  | #  -------------------------------------------
  | #
  | >  flipper_config = ENV['NO_FLIPPER'] == "1" ? FlipperConfiguration.disabled : FlipperConfiguration.enabled
  | #
  | #  -------------------------------------------

I see. I hadn't seen those commands in the pipeline logs that were erroring and then succeeded with the new changes; otherwise, I would have handled these issues. Since I just joined, I have been trying to understand the process here. Would you know if there is any other command in the pipeline besides cd playground/android && ./gradlew testReactNative71ReleaseUnitTest that would cause problems? Let me know so we can remove back and forth, and I can understand the entire process of having a fully ready PR and continuing to contribute, potentially for v75.

I will be tackling these issues this week after working hours.

@d4vidi
Copy link
Collaborator

d4vidi commented Aug 28, 2024

All relevant scripts are available as npm scripts in package.json (at least the entry points...)

As for build commands, my advice is to focus on the basics (e.g. a npm build) and then follow the Detox configuration (i.e. the detox section in package.json):

    "apps": {
      "ios.debug": {
        "type": "ios.app",
        "binaryPath": "playground/ios/DerivedData/playground/Build/Products/Debug-iphonesimulator/playground.app",
        "start": "npm start -- --e2e",
        "build": "RCT_NO_LAUNCH_PACKAGER=true xcodebuild build -scheme playground -workspace playground/ios/playground.xcworkspace -sdk iphonesimulator -configuration Debug -derivedDataPath playground/ios/DerivedData/playground ONLY_ACTIVE_ARCH=YES -quiet -UseModernBuildSystem=YES"
      },
      "ios.release": {
        "type": "ios.app",
        "binaryPath": "playground/ios/DerivedData/playground/Build/Products/Release-iphonesimulator/playground.app",
        "build": "RCT_NO_LAUNCH_PACKAGER=true xcodebuild build -scheme playground_release -workspace playground/ios/playground.xcworkspace -sdk iphonesimulator -configuration Release -derivedDataPath playground/ios/DerivedData/playground ONLY_ACTIVE_ARCH=YES -quiet -UseModernBuildSystem=YES"
      },
      "android.debug": {
        "type": "android.apk",
        "binaryPath": "playground/android/app/build/outputs/apk/debug/app-debug.apk",
        "start": "npm start -- --e2e",
        "build": "cd playground/android && ./gradlew app:assembleDebug app:assembleAndroidTest -DtestBuildType=debug"
      },
      "android.release": {
        "type": "android.apk",
        "binaryPath": "playground/android/app/build/outputs/apk/release/app-release.apk",
        "build": "cd playground/android && ./gradlew app:assembleRelease app:assembleAndroidTest -DtestBuildType=release"
      }
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants