Add getMessage to JThrowable and getSimpleName to JClass

krystofwoldrich commented 1 year ago

Motivation

Why are you making this change?

To enable RN to retrieve details about Throwables.

Relates to https://github.com/facebook/react-native/pull/36925. Where these changes will be used.

Summary

What did you change?

Added getMessage to JThrowable.

Added getSimpleName to JClass.

How does the code work?

Uses dynamic Java method call like other similar methods.

Test Plan

How did you test this change?

Could anyone guide me on how to test my changes of fbjni in RN?

I wanted to publish the package to my local Maven and add it to RN to verify the functionality, but it didn't work.

What I tried:

./gradlew publishToMavenLocal generated 0.3.1-SNAPSHOT
Add mavenLocal() to packages/rn-tester/android/app/build.gradle.
yarn install-android-jsc

I got this error:

[CXX1429] error when building with cmake using /Users/krystofwoldrich/random/react-native/packages/react-native/ReactAndroid/src/main/jni/CMakeLists.txt: C++ build system [prefab] failed while executing:
    "/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java" \
      --class-path \
      /Users/krystofwoldrich/.gradle/caches/modules-2/files-2.1/com.google.prefab/cli/2.0.0/f2702b5ca13df54e3ca92f29d6b403fb6285d8df/cli-2.0.0-all.jar \
      com.google.prefab.cli.AppKt \
      --build-system \
      cmake \
      --platform \
      android \
      --abi \
      arm64-v8a \
      --os-version \
      21 \
      --stl \
      c++_shared \
      --ndk-version \
      23 \
      --output \
      /var/folders/tl/jddrmdy97gj0cljrcwb_qkzc0000gn/T/agp-prefab-staging13347947173282983619/staged-cli-output \
      /Users/krystofwoldrich/.gradle/caches/transforms-3/f866505a036566e921ad0564b1cacdd9/transformed/fbjni-0.3.1-SNAPSHOT/prefab \
      /Users/krystofwoldrich/random/react-native/packages/react-native/ReactAndroid/build/intermediates/cxx/refs/packages/react-native/ReactAndroid/hermes-engine/572y156m
  from /Users/krystofwoldrich/random/react-native/packages/react-native/ReactAndroid
Exception in thread "main" java.lang.IllegalArgumentException: version must be compatible with CMake, if present
    at com.google.prefab.api.Package.<init>(Package.kt:58)
    at com.google.prefab.cli.Cli$packages$2.invoke(Cli.kt:124)
    at com.google.prefab.cli.Cli$packages$2.invoke(Cli.kt:123)
    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
    at com.google.prefab.cli.Cli.getPackages(Cli.kt:123)
    at com.google.prefab.cli.Cli.validate(Cli.kt:172)
    at com.google.prefab.cli.Cli.run(Cli.kt:189)
    at com.github.ajalt.clikt.parsers.Parser.parse(Parser.kt:204)
    at com.github.ajalt.clikt.parsers.Parser.parse(Parser.kt:17)
    at com.github.ajalt.clikt.core.CliktCommand.parse(CliktCommand.kt:396)
    at com.github.ajalt.clikt.core.CliktCommand.parse$default(CliktCommand.kt:393)
    at com.github.ajalt.clikt.core.CliktCommand.main(CliktCommand.kt:411)
    at com.github.ajalt.clikt.core.CliktCommand.main(CliktCommand.kt:436)
    at com.google.prefab.cli.AppKt.main(App.kt:28)

Any change that adds functionality should add a unit test as well.

cortinico commented 1 year ago

@passy are you able to take this one?

passy commented 1 year ago

@cortinico I don't know the internals of this well enough (i.e. not at all) but I'm happy to import it and try to find an internal reviewer.

lblasa commented 1 year ago

@cortinico I don't know the internals of this well enough (i.e. not at all) but I'm happy to import it and try to find an internal reviewer.

I can import and review the changes 😊

cortinico commented 1 year ago

@cortinico I don't know the internals of this well enough (i.e. not at all) but I'm happy to import it and try to find an internal reviewer.

I can import and review the changes 😊

Thanks @lblasa I can help with the review

krystofwoldrich commented 1 year ago

Thank you for the help. @cortinico @passy @lblasa

Would anyone know how to build the library locally and try it with RN? I've described my steps in the description. I thought I'll build it and publish it to my local maven and then use it RN, but got an error mentioned above.

cortinico commented 1 year ago

Would anyone know how to build the library locally and try it with RN? I've described my steps in the description. I thought I'll build it and publish it to my local maven and then use it RN, but got an error mentioned above.

Answered on Discord but I'll follow up here also for completeness:

Seems like prefab support is not working for -SNAPSHOT version (here the problem https://github.com/DanAlbert/prefab/blob/ab8529aa5163f071da02deb023037f3511bf90ce/api/src/main/kotlin/com/google/prefab/api/Package.kt#L31-L32).

It seems like this might be fixed in a later version of AGP though so there is probably nothing to report to Google.

To unblock you can

You need to bump the version of FBJNI to 0.3.2 in the gradle.properties
You can publish locally with ./gradlew publishToMavenLocal -x signMavenPublication
And you can just run the react native build with mavenLocal() set up in the repositories{} block