Finish the Java-facing side of the TurboModule interop layer#36627
Closed
RSNara wants to merge 6 commits into
Closed
Finish the Java-facing side of the TurboModule interop layer#36627RSNara wants to merge 6 commits into
RSNara wants to merge 6 commits into
Conversation
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
Base commit: 4e0dfed |
RSNara
force-pushed
the
export-D43751055
branch
from
9998e5d to
cc1118c
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
1 similar comment
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
2 times, most recently
from
cd446b5 to
d1b4112
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
1 similar comment
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
d1b4112 to
46d6719
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
46d6719 to
4329c1c
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
4329c1c to
78818d6
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
78818d6 to
3df6c61
Compare
RSNara
force-pushed
the
export-D43751055
branch
from
3df6c61 to
fd8b3eb
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
1 similar comment
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
fd8b3eb to
95e86ad
Compare
RSNara
force-pushed
the
export-D43751055
branch
from
09dc273 to
81b11ec
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
81b11ec to
2a798be
Compare
RSNara
force-pushed
the
export-D43751055
branch
from
2a798be to
51f6ead
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
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: 0ccfddbf82759bdc8ea26a9ba82f3d005ceed48a
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: 3fc1848533dc027e17bee2dfda619b848f032e1e
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: 50b6691c62f1d2b9e8cad92cbde27174a2a16025
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: f44538959fa108c3f7fd813ae4908c73ccc25b51
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: c951a5fd72325f30a35ed4a05f94fe434b1a7969
…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] Reviewed By: cortinico Differential Revision: D43751055 fbshipit-source-id: 25da5ccfbbbff5e3d8aaffba97b3dd846a5a68cf
RSNara
added a commit
to RSNara/react-native
that referenced
this pull request
Mar 28, 2023
…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
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D43751055 |
RSNara
force-pushed
the
export-D43751055
branch
from
51f6ead to
6da319e
Compare
RSNara
added a commit
to RSNara/react-native
that referenced
this pull request
Mar 28, 2023
…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: b96264f76626a5378367219e2a49a18b16d3e3bc
Contributor
|
This pull request has been merged in 185bc24. |
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:
Changes
Now, when you call TurboModuleManager.getModule(interopModuleName), the TurboModuleManager will create and return that interop module to you.
Changes in this diff:
Details
TurboModuleManagerDelegate's capabilities:
||API| Without Interop | With Interop |
|same|getModule()|Java NativeModule that also implements TurboModule |Java NativeModule that also implements TurboModule |
|changed|getLegacyCxxModule()|Java CxxModuleWrapper that implements TurboModule |Java CxxModuleWrapper that doesn't necessarily implement TurboModule|
|new|getLegacyJavaModule()| |Java NativeModule that doesn't implement TurboModule|
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D43751055