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

Upgrade to react-native 0.67 #16038

Merged
merged 9 commits into from Feb 17, 2022
Merged

Upgrade to react-native 0.67 #16038

merged 9 commits into from Feb 17, 2022

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Jan 24, 2022

Why

try to bump react-native version for sdk 45

How

Test Plan

  • bare-expo launch

Checklist

  • Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md).
  • This diff will work correctly for expo build (eg: updated @expo/xdl).
  • This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

@Krisztiaan
Copy link

[bare-expo][android] maven plugin from third-party: react-native-appearance. consider it's archived and deprecated, how to deal with this.

Why is this necessary to keep, when react-native has it's own Appearance now?

@Kudo
Copy link
Contributor Author

Kudo commented Jan 27, 2022

@Krisztiaan it's not necessary as we are going to deprecate expo sdk 42. i'll remove it later. thanks!

@Krisztiaan
Copy link

Krisztiaan commented Jan 27, 2022

@Kudo cliPath seems broken with react-native. Omitting it by default, or using a relative path solves this issue.

react-native/react.gradle introduced a change to break it, released in 0.67.1

if (config.cliPath) {
    return "${projectDir}/${config.cliPath}"
}

@Krisztiaan
Copy link

I have created a PR on react-native to address this where it was broken.

@Kudo
Copy link
Contributor Author

Kudo commented Jan 28, 2022

that's great. thanks @Krisztiaan!

Kudo added a commit that referenced this pull request Jan 28, 2022
…16099)

# Why

fix crash when `enableHermes: true` with dev-menu on react-native 0.67. i tested it from bare-expo.
stacktrace:

```
FATAL EXCEPTION: main
Process: dev.expo.payments, PID: 18344
java.lang.NoClassDefFoundError: com.facebook.react.jscexecutor.JSCExecutor
    at com.facebook.react.jscexecutor.JSCExecutor.loadLibrary(JSCExecutor.java:24)
    at com.facebook.react.ReactInstanceManagerBuilder.getDefaultJSExecutorFactory(ReactInstanceManagerBuilder.java:352)
    at com.facebook.react.ReactInstanceManagerBuilder.build(ReactInstanceManagerBuilder.java:319)
    at com.facebook.react.ReactNativeHost.createReactInstanceManager(ReactNativeHost.java:95)
    at expo.modules.devmenu.DevMenuHost.createReactInstanceManager(DevMenuHost.kt:41)
    at com.facebook.react.ReactNativeHost.getReactInstanceManager(ReactNativeHost.java:42)
    at expo.modules.devmenu.DevMenuManager.maybeInitDevMenuHost$lambda-8(DevMenuManager.kt:169)
    at expo.modules.devmenu.DevMenuManager.lambda$rZDvUiNHHHvkcRmdKO265huCv6U(Unknown Source:0)
    at expo.modules.devmenu.-$$Lambda$DevMenuManager$rZDvUiNHHHvkcRmdKO265huCv6U.run(Unknown Source:0)
    at android.os.Handler.handleCallback(Handler.java:938)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:223)
    at android.app.ActivityThread.main(ActivityThread.java:7656)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
Caused by: java.lang.UnsatisfiedLinkError: couldn't find DSO to load: libjscexecutor.so
    SoSource 0: com.facebook.soloader.ApkSoSource[root = /data/data/dev.expo.payments/lib-main flags = 1]
    SoSource 1: com.facebook.soloader.DirectorySoSource[root = /data/app/~~ibMyBupkSRJXdt_c772rhw==/dev.expo.payments-ru7gzQ9g2MilqIhz24Lduw==/lib/arm64 flags = 0]
    SoSource 2: com.facebook.soloader.DirectorySoSource[root = /vendor/lib64 flags = 2]
    SoSource 3: com.facebook.soloader.DirectorySoSource[root = /system/lib64 flags = 2]
    Native lib dir: /data/app/~~ibMyBupkSRJXdt_c772rhw==/dev.expo.payments-ru7gzQ9g2MilqIhz24Lduw==/lib/arm64
```

the first time to create a `ReactInstanceManager`, it successes. however, the second time to create a `ReactInstanceManager` for dev-menu, it crashes with `NoClassDefFoundError`. it's also uncaughtable [here](https://github.com/facebook/react-native/blob/20b9ed91c02a0cbf2f970bf51ccc4c278c63540c/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java#L348-L354) 

this is because the change landed in react-native 0.67: facebook/react-native#30749

according to jvm spec v2 2.17.5, ([maybe from this spec](https://docs.oracle.com/javase/specs/jvms/se6/html/Concepts.doc.html#24237)). the second time to load a failure class, the vm throws `NoClassDefFoundError`. [there's another reference in android art vm implementation](https://android.googlesource.com/platform/art/+/ec696e5d98ae4c503966f199bac341b50bfa1a5b/runtime/class_linker.cc#478)

# How

for dev-menu, we try to locate `libjsc.so` and create specific javascript executor.

# Test Plan

tested on my react-native 0.67 upgrade branch: #16038
@Simek
Copy link
Collaborator

Simek commented Jan 29, 2022

Fixes #16040

For unknow for me reason I wasn't able to link the issue from the UI, so I need to leave a comment. 🤷‍♂️

@hirbod
Copy link
Contributor

hirbod commented Feb 2, 2022

RN 0.67.2 is already out (+ fixing CVE-2021-0341)

@Kudo
Copy link
Contributor Author

Kudo commented Feb 14, 2022

ci error from #16145, maybe @Simek could help to take a look when you get a chance. i am going to set this pr ready for review even facebook/react-native#32983 is not landed yet.

@Kudo Kudo marked this pull request as ready for review February 14, 2022 15:31
@Kudo Kudo requested a review from ide as a code owner February 14, 2022 15:31
@Kudo Kudo removed the request for review from ide February 14, 2022 15:31
Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking dope 🚀
Superb work on upgrading the RN to the newest version 👏

Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like tests will pass after rebasing on top of #16300 so :shipit:

@Kudo
Copy link
Contributor Author

Kudo commented Feb 17, 2022

just rebased and will merge after ci passing.

@expo-bot
Copy link
Collaborator

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing changelog entries


Your changes should be noted in the changelog. Read Updating Changelogs guide and consider (it's optional) adding an appropriate entry to the following changelogs:


Generated by ExpoBot 🤖 against dda36b0

@Kudo Kudo merged commit f5787ad into main Feb 17, 2022
@Kudo Kudo deleted the @kudo/rn067 branch February 17, 2022 16:55
prakashbask pushed a commit to prakashbask/expo that referenced this pull request Mar 16, 2022
# Why

try to bump react-native version for sdk 45

# How

- [x] upgrade package.json version to `react-native@0.67.2`
- [x] apply changes from [upgrade helper](https://react-native-community.github.io/upgrade-helper/?from=0.66.4&to=0.67.2)
- [x] since 0.67 upgrade to gradle 7 and remove `maven` plugin. ~this pr also migrate packages to use `maven-publish` plugin~ landed in separated pr expo#16080
- [x] [bare-expo][android] maven plugin from third-party: `react-native-appearance`. ~consider it's archived and deprecated, how to deal with this.~ since we are going to drop sdk 42, i've removed this dependency.
- [x] [bare-expo][android] maven plugin from third-party: `react-native-shared-element`. IjzerenHein/react-native-shared-element#90
- [x] [dev-launcher][android] having a `DevLauncherDevSupportManager.kt` for 0.67 and passing null to the new `SurfaceDelegateFactory`. simply to fix the build error first.
- [x] [dev-menu][android] it will crash from missing libjsc.so in hermes mode. ~workaround to use jsc mode first~. fix pr: expo#16099

# Test Plan

- bare-expo launch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: suggestions ExpoBot has some suggestions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants