-
Notifications
You must be signed in to change notification settings - Fork 906
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: autolinking for the new architecture on Android #1603
Conversation
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.
Did a first pass. left a couple of minor comments.
Q: Are those:
libraryName?: string;
componentDescriptors?: string[];
androidMkPath?: string;
configurable via npx react-native config
?
rncliCppIncludes = packages.collect { | ||
def result = "#include <${it.libraryName}.h>" | ||
if (it.componentDescriptors.size() > 0) { | ||
result += "\n#include <react/renderer/components/${it.libraryName}/ComponentDescriptors.h>" |
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.
Two things here:
- Please export to a constant the
react/renderer/components/
andComponentDescriptors.h
- There might be some escaping to do. I'll check and get back to you
|
||
task generatePackageList { | ||
doLast { | ||
autoModules.generatePackagesFile(generatedCodeDir, generatedFileName, generatedFileContentsTemplate) | ||
} | ||
} | ||
|
||
task generateNewArchitectureFiles { | ||
doLast { |
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.
I would refrain from using doLast
. Those are terrible from the caching point of view (as they invalidate the gradle caches). We should move this to a proper task.
Not a blocker though :), just a heads up
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.
Sure! I'm not a heavy user of Gradle, so I'd appreciate any contributions in code or pointers on what to leverage exactly :)
EDIT: it's now possible to override both from library or app config. |
Made it work for libraries with their own autolinking setup (RNScreens, RNSafeAreaContext). This is ready to be published in next major version. Planned for this week. |
1883f11
to
58b7122
Compare
Released in v9.0.0-alpha.0 |
Summary: Provides necessary changes for the autolinking to work in new architecture on Android. Depends on react-native-community/cli#1603 and is subject to change. Upgraded the RN CLI to v9.0.0-alpha.0 so that it's testable locally. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Android] [Change] - Adapt template to new architecture autolinking on Android Pull Request resolved: #33777 Test Plan: CI Reviewed By: cipolleschi Differential Revision: D36478984 Pulled By: cortinico fbshipit-source-id: 970fa7bcb77898d9defae18c20026a7783ba4108
Summary:
This PR introduces an initial support for autolinking the new architecture for Android platform. iOS is already handled AFAIK.
Closes #1596
High-level overview of how it works
When the new architecture is turned on, the
generateNewArchitectureFiles
task is fired, generating/android/build/generated/rn/src/main/jni
directory with the following files:Android-rncli.mk
– creates a list of codegen'd libs. Used by the project'sAndroid.mk
.rncli.cpp
– registers codegen'd Turbo Modules and Fabric component providers. Used byMainApplicationModuleProvider.cpp
andMainComponentsRegistry.cpp
.rncli.h
- a header file forrncli.cpp
.This change requires updating the default template in react-native repository. PR: facebook/react-native#33777
Test Plan:
So far I've been testing it with following libraries:
react-native-screens
,react-native-safe-area-context
through React Navigation – RNScreens provides a custom setup which bypasses autolinking through obfuscating its configuration. To have it compile we need to opt-out from autolinking by nulling added properties, like so:How to test it:
android/build/generated/rn/src/main/jni
is populated withAndroid-rncli.mk
,rncli.cpp
andrncli.h
, correctly referencing your library.yarn react-native config
contains 3 newly added config keys with correct values. E.g.: