Skip to content

Provide defaults for TurboModuleManagerDelegate and JSIModulePackage#34418

Closed
cortinico wants to merge 1 commit into
mainfrom
nc/encapsulate-rn-tester
Closed

Provide defaults for TurboModuleManagerDelegate and JSIModulePackage#34418
cortinico wants to merge 1 commit into
mainfrom
nc/encapsulate-rn-tester

Conversation

@cortinico
Copy link
Copy Markdown
Contributor

@cortinico cortinico commented Aug 15, 2022

Summary

This is an attempt to relax the need of specifying a custom TurboModuleManagerDelegate and a JSIModulePackage in new apps for New Architecture.

Users can just specify the name of the dynamic library to load and they'll default to use the DefaultTurboModuleManagerDelegate and DefaultJSIModulePackage.
Users will still have to provide a C++ implementation for it for the time being, but this at least removes one extra file that we requested them to create and maintain.

If we're fine with this approach, I'll replicate it inside the default template.

Changelog

[Android] [Added] - Provide defaults for TurboModuleManagerDelegate and JSIModulePackage

Test Plan

RN Tester works, given you SoLoader.loadLibrary(BuildConfig.DYNAMIC_LIBRARY_NAME) in onCreate as otherwise the implementation for DefaultTurboModuleManagerDelegate.initHybrid can't be found.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels Aug 15, 2022
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot react-native-bot added Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Aug 15, 2022
@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Aug 15, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: d35aab4
Branch: main

Summary:
This is an attempt to relax the need of specifying a custom `TurboModuleManagerDelegate` and a `JSIModulePackage` in new apps for New Architecture.
Users can just specify the name of the dynamic library to load and they'll default to use the `DefaultTurboModuleManagerDelegate` and `DefaultJSIModulePackage`.

Users will still have to provide a C++ implementation for it for the time being, but this at least removes
one extra file that we requested them to create and maintain.

If we're fine with this approach, I'll replicate it inside the default template.

Changelog:

[Android] [Added] - Provide defaults for TurboModuleManagerDelegate and JSIModulePackage

Reviewed By: cipolleschi

Differential Revision: D38701180

fbshipit-source-id: 68f2193a2a414bdde7c3b35081f7155a7983fa4a
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D38701180

@cortinico cortinico force-pushed the nc/encapsulate-rn-tester branch from 6470e39 to 417d3eb Compare August 16, 2022 13:39
@cortinico cortinico changed the title Provide a default TurboModuleManagerDelegate Provide defaults for TurboModuleManagerDelegate and JSIModulePackage Aug 16, 2022
@cortinico cortinico marked this pull request as ready for review August 16, 2022 13:44
@analysis-bot
Copy link
Copy Markdown

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,611,261 -252
android hermes armeabi-v7a 7,025,957 -145
android hermes x86 7,910,728 -444
android hermes x86_64 7,884,773 -306
android jsc arm64-v8a 9,490,273 -200
android jsc armeabi-v7a 8,267,846 -88
android jsc x86 9,427,321 -393
android jsc x86_64 10,020,475 -240

Base commit: d35aab4
Branch: main

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @cortinico in 9a2eb90.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 16, 2022
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
…acebook#34418)

Summary:
Pull Request resolved: facebook#34418

This is an attempt to relax the need of specifying a custom `TurboModuleManagerDelegate` and a `JSIModulePackage` in new apps for New Architecture.
Users can just specify the name of the dynamic library to load and they'll default to use the `DefaultTurboModuleManagerDelegate` and `DefaultJSIModulePackage`.

Users will still have to provide a C++ implementation for it for the time being, but this at least removes
one extra file that we requested them to create and maintain.

If we're fine with this approach, I'll replicate it inside the default template.

Changelog:

[Android] [Added] - Provide defaults for TurboModuleManagerDelegate and JSIModulePackage

Reviewed By: cipolleschi

Differential Revision: D38701180

fbshipit-source-id: eec302c5789990700eb75353d97751358ca6799f
@yungsters yungsters deleted the nc/encapsulate-rn-tester branch September 29, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants