-
Notifications
You must be signed in to change notification settings - Fork 96
feat: add support for TurboModule #843
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
Conversation
f575155 to
1ee3eac
Compare
53bd827 to
6ab1ccf
Compare
133eada to
033e6fe
Compare
033e6fe to
0ab7370
Compare
eeef457 to
2e7ee77
Compare
|
couple of quick things about the test steps:
Now, talking about testing the example app:
|
Oops… description should be updated now.
Correct. I wanted a focus on the flags that are the same in core. Less to teach.
This shouldn't be broken. I'll have a look.
This is expected. TM depends on code that is under the Fabric flag.
Unfortunately, yes.
This is expected. TM depends on code that is under the Fabric flag. |
2e7ee77 to
6dbe56b
Compare
Note that this requires some adjustments to your `build.gradle`: ```diff diff --git a/example/android/build.gradle b/example/android/build.gradle index 0c6c69d..2ed70f1 100644 --- a/example/android/build.gradle +++ b/example/android/build.gradle @@ -1,6 +1,7 @@ buildscript { def androidTestAppDir = "../node_modules/react-native-test-app/android" apply(from: "${androidTestAppDir}/dependencies.gradle") + apply(from: "${androidTestAppDir}/test-app-util.gradle") repositories { mavenCentral() @@ -8,8 +9,12 @@ buildscript { } dependencies { - classpath "com.android.tools.build:gradle:$androidPluginVersion" - classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" + classpath("com.android.tools.build:gradle:${androidPluginVersion}") + + if (isNewArchitectureEnabled(project)) { + classpath("com.facebook.react:react-native-gradle-plugin") + classpath("de.undercouch:gradle-download-task:5.1.0") + } } } ``` If you're seeing build issues with `newArchEnabled=true`, please see https://github.com/microsoft/react-native-test-app/wiki/Troubleshooting.
6dbe56b to
ce4d31d
Compare
da7c91c to
394fea7
Compare
394fea7 to
4f19883
Compare
kelset
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
### Platforms Impacted - [x] iOS - [x] macOS - [ ] win32 (Office) - [ ] windows - [x] android ### Description of changes Let's bump us to the latest release of react-native-test-app, which gets us a lot of goodies :) 1) Turbomodule support. microsoft/react-native-test-app#843 2) App icon support. microsoft/react-native-test-app#1022 3) iPad Slide Over window / multi-tasking support. microsoft/react-native-test-app#1032 We also got a designer to make us icons =). I chose the blue set over the pink set. ### Verification I didn't test building with turbo modules, we don't quite support this yet anyway. Verified that SlideOver windows work on iOS. Some additional JS fixes are needed to make layout act properly though. Android icon:  macOS icon (dock):  iOS icon  ### Pull request checklist This PR has considered (when applicable): - [ ] Automated Tests - [ ] Documentation and examples - [ ] Keyboard Accessibility - [ ] Voiceover - [ ] Internationalization and Right-to-left Layouts






Description
Adds support for TurboModule. I'm still seeing issues with Gradle not building things in the correct order. Until a new version of Gradle is released, we will have to point people at Android NDK: ERROR:/~/android/src/main/jni/Android.mk:fb: LOCAL_SRC_FILES points to a missing file.
Resolves #836.
Blocked by
Autolinking support on Android: TurboModule Deep Dive reactwg/react-native-new-architecture#2 (reply in thread)Implemented in feat: autolinking for the new architecture on Android react-native-community/cli#1603Platforms affected
Test plan
react-native0.69:yarn set-react-version 0.69@react-native-community/cli9.0:yarnafter making the changes aboveyarn androidRCT_NEW_ARCH_ENABLED=1 pod installyarn ios