Setup TurboModule interop test in Fb4a#36669
Closed
RSNara wants to merge 8 commits into
Closed
Conversation
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
1 similar comment
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
6ab9166 to
6a54211
Compare
Base commit: 4e0dfed |
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
6a54211 to
7767011
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
7767011 to
d61358d
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
d61358d to
98964d4
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
98964d4 to
2abf455
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
2abf455 to
a466ac8
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
a466ac8 to
b948155
Compare
Summary: In Bridgeless mode, With the TurboModule interop layer, the TurboModule system will need to customize the nativeModuleProxy global. This customization would be much easier if the nativeModuleProxy global were installed by the TurboModule system (and not the Bridgeless core). So, this diff moves nativeModuleProxy installation into the TurboModule system. Changelog: [Internal] Differential Revision: https://internalfb.com/D43993197 fbshipit-source-id: 255278ee4b85630b4f0b44a5a8149635b711c60c
Summary: ## Context Previously, jsRepresentation would only cache the **HostFunctions** returned from TurboModule::createHostFunction(). ## Changes This diff replaces TurboModule::createHostFunction() with TurboModule::create(). Now, jsRepresentation will cache **all** the **properties** returned from TurboModule::create(). ## Motivation For interop modules, constants will be exported as properties on the TurboModule HostObject. This diff allows those constants (which are non HostFunctions) to be cached. Changelog: [Internal] Differential Revision: https://internalfb.com/D44253229 fbshipit-source-id: 50e44c4d25e1d08d76dc439460aa7309a9ce01bb
Summary: The legacy NativeModule system supports integer and float in NativeModule method arguments and returns. This diff extends the TurboModule system for the same functionality. T his is necessary because the TurboModule system will now need to dispatch method calls to legacy NativeModules. NOTE: We can't actually test these changes until we run interop modules. Changelog: [Internal] Differential Revision: https://internalfb.com/D44000389 fbshipit-source-id: 8b54f61256a0b37cb258fbfff31f29d753271cc1
Summary: The scope of TurboModuleManager is increasing: - Eventually, it'll be capable of creating interop NativeModules (i.e: NativeModules that don't implement TurboModule). So, instead of creating duplicate methods for NativeModules on the TurboModuleManager, this diff changes the APIs of TurboModuleManager to work with the NativeModule interface. Thoughts? ## Questions **Question:** Is this a breaking change for open source? - Technically, yes. This diff changes the public interface of TurboModuleManager. **Question:** How large of a thrash will this cause for open source apps? - The thrash should be minimal. People in open source shouldn't be creating their own TurboModuleManager. They also shouldn't be directly accessing the TurboModuleManager object either. **Question:** Is this change safe? - Yeah. All the code that calls into TurboModuleRegistry converts TurboModules it returns into NativeModules. **Question:** Is this change move us in the right direction? - Long term, the TurboModule system will support legacy modules as well as TurboModules. - I think it makes a lot of sense to have one Java-facing registry: after all, Java will just treat these NativeModules/TurboModules as regular Java objects, and call public methods on them. It doesn't care if the module is TurboModule-compatible or not. - As for the TurboModuleRegistry abstraction, I think we should eventually rename this to NativeModuleRegistry after we delete the current NativeModuleRegistry. - Still thinking about this though. I will leave this diff in review to welcome comments. Changelog: [Android][Deprecated] - Deprecate TurboModuleRegistry.getModule(), getModules(), hasModule(), Differential Revision: https://internalfb.com/D43801531 fbshipit-source-id: d38248f45c4173f934233d2105af5691efce343d
Summary: ## Context TurboModuleManagerDelegate exposes two methods that create TurboModules: - TurboModule getModule() - CxxModuleWrapper getLegacyCxxModule() ## Problem TurboModuleManagerDelegate.getLegacyCxxModule() is redundant: getModule() could just return all the modules that getLegacyCxxModule() returns: getLegacyCxxModule returns modules that implement TurboModule. ## Changes So, let's deprecate getLegacyCxxModule(). This will simplify the implementation of TurboModuleManager. Changelog: [Android][Deprecated] - Deprecate TurboModuleManager.getLegacyCxxModule Differential Revision: https://internalfb.com/D44407802 fbshipit-source-id: da79341ce4f559605c3c5c6773f950fb38521744
…k#36627) Summary: Pull Request resolved: facebook#36627 ## Changes Now, when you call TurboModuleManager.getModule(interopModuleName), the TurboModuleManager will create and return that interop module to you. Changes in this diff: 1. Forward interop NativeModules from app's ReactPackages to the TurboModuleManager 2. Extend TurboModule system's module creation algorithm to create interop NativeModules. ## Details TurboModuleManagerDelegate's capabilities: ||API| Without Interop | With Interop | |same|getModule()|Java [NativeModule](https://www.internalfb.com/code/fbsource/[e5db2a0dc412f0656f7eeec1db9d2da4aab61f40]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java?lines=21) that also implements [TurboModule](https://www.internalfb.com/code/fbsource/[c7089c1408eda109f342a1f33252533e743614ed]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/interfaces/TurboModule.java?lines=11) |Java [NativeModule](https://www.internalfb.com/code/fbsource/[e5db2a0dc412f0656f7eeec1db9d2da4aab61f40]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java?lines=21) that also implements [TurboModule](https://www.internalfb.com/code/fbsource/[c7089c1408eda109f342a1f33252533e743614ed]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/interfaces/TurboModule.java?lines=11) | |new|getLegacyModule()| |Java [NativeModule](https://www.internalfb.com/code/fbsource/[e5db2a0dc412f0656f7eeec1db9d2da4aab61f40]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java?lines=21) that **doesn't** implement [TurboModule](https://www.internalfb.com/code/fbsource/[c7089c1408eda109f342a1f33252533e743614ed]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/interfaces/TurboModule.java?lines=11)| Changelog: [Internal] Differential Revision: https://internalfb.com/D43751055 fbshipit-source-id: 257310df64f90fd7085765c36efcae2ed59e8484
…36630) Summary: Pull Request resolved: facebook#36630 ## Changes This diff hooks up global.nativeModuleProxy to the TurboModule interop layer. Now, when you call NativeModules.Foo, the TurboModule system will create the Foo interop module, and return it to JavaScript. |**Language**|**Abstraction**|**Description**| |Java/C++|MethodDescriptor| The information needed by JavaTurboModule::invokeJavaMethod() to execute a module method: [example](https://www.internalfb.com/code/fbsource/[78577e97310db97c489e976168ca6ddf4cb894c3]/xplat/js/react-native-github/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp?lines=34-36).| |Java|TurboModuleInteropUtils| Takes the interop module object, and parses out MethodDescriptors from the methods annotated with ReactMethod| |C++|JavaInteropTurboModule| Facilitates JavaScript -> Java method dispatch for interop modules. Extends [JavaTurboModule](https://www.internalfb.com/code/fbsource/[6f0698784af39dd0e881d9a69087ae6ac5e9cdc4]/xplat/js/react-native-github/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h?lines=28). Needs to be created with a list of MethodDescriptors.| Shape of MethodDescriptor: ``` class MethodDescriptor { String methodName; String jniSignature; String jsiReturnKind; int jsArgCount; } ``` ## Example global.nativeModuleProxy.Foo: 1. **Java:** Use TurboModuleManager to create the Java interop module for Foo 2. **Java:** Use TurboModuleInteropUtils to generate Foo's MethodDescriptors 3. **C++:** Use Foo's MethodDescriptors to create, cache, and return a JavaInteropTurboModule object to JavaScript. Changelog: [Internal] Differential Revision: https://internalfb.com/D43918998 fbshipit-source-id: 68ddd2da61bd670f613e373433ce1eeb91f23cb7
Summary: Pull Request resolved: facebook#36669 The test: Route all TurboModules in Fb4a through the interop layer. So now, instead of using the C++ codegen for method dispatch, TurboModules will instead be using JavaInteropTurboModule, which uses the ReactMethod annotations for method dispatch. MobileConfig actualization diff: D44405316. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D44405336 fbshipit-source-id: d3712c41ebd8117633d5afbfec2e818e8a591e61
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D44405336 |
RSNara
force-pushed
the
export-D44405336
branch
from
b948155 to
3419dd8
Compare
Contributor
|
This pull request has been merged in 8305694. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
The test: Route all TurboModules in Fb4a through the interop layer.
So now, instead of using the C++ codegen for method dispatch, TurboModules will instead be using JavaInteropTurboModule, which uses the ReactMethod annotations for method dispatch.
MobileConfig actualization diff: D44405316.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D44405336