Skip to content

Proposal about 0002-Turbo-Modules.md #11

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

Merged
merged 4 commits into from
Oct 11, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions proposals/0002-Turbo-Modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@ _Its blazing fast_

## Summary

React Native uses [Native Modules](https://facebook.github.io/react-native/docs/native-modules-ios) as a way for JavaScript to invoke functions in Java/ObjC. Listed unde the API section in the [React Native documentation](https://facebook.github.io/react-native/docs/0.56/getting-started), popular native modules include Geolocation, Device Information, Async Storage, etc.
This proposal discusses ways to speed up the way Native Modules are initialized and invoked from JavaScript.
React Native uses [Native Modules](https://facebook.github.io/react-native/docs/native-modules-ios) as a way for JavaScript to invoke functions in Java/ObjC. Listed under the API section in the [React Native documentation](https://facebook.github.io/react-native/docs/0.56/getting-started), popular native modules include Geolocation, Device Information, Async Storage, etc.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the getting-started link 404s

This proposal discusses a new architecture to speed up the way Native Modules are initialized and invoked from JavaScript.

## Motivation

This section lists the problems with the current state. Though Android is used as the example, similar issues exist in iOS also.
Here are some issues with the current state. Though Android is used as the example, the narrative applies to iOS also.

1. Native Modules are specified in [a package](https://github.com/facebook/react-native/blob/1e8f3b11027fe0a7514b4fc97d0798d3c64bc895/local-cli/link/__fixtures__/android/0.20/MainActivity.java#L37) and are eagerly initialized. The startup time of React Native increases with the number of Native Modules, even if some of those Native Modules are never used. Native Modules can be made lazy by using [LazyReactPackage](https://github.com/facebook/react-native/blob/42146a7a4ad992a3597e07ead3aafdc36d58ac26/ReactAndroid/src/main/java/com/facebook/react/LazyReactPackage.java) but this does not work in the Open Source repo since work since the annotation processor called [ReactModuleSpecProcessor](https://github.com/facebook/react-native/blob/42146a7a4ad992a3597e07ead3aafdc36d58ac26/ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java) does not work with Gradle yet. Note that there was [a PR to solve this](https://github.com/facebook/react-native/pull/10084), but it was not merged.
1. Native Modules are specified in [a package](https://github.com/facebook/react-native/blob/1e8f3b11027fe0a7514b4fc97d0798d3c64bc895/local-cli/link/__fixtures__/android/0.20/MainActivity.java#L37) and are eagerly initialized. The startup time of React Native increases with the number of Native Modules, even if some of those Native Modules are never used. Native Modules can be initialized lazily using [LazyReactPackage](https://github.com/facebook/react-native/blob/42146a7a4ad992a3597e07ead3aafdc36d58ac26/ReactAndroid/src/main/java/com/facebook/react/LazyReactPackage.java) but this does not work in the Open Source repo as the annotation processor [ReactModuleSpecProcessor](https://github.com/facebook/react-native/blob/42146a7a4ad992a3597e07ead3aafdc36d58ac26/ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java) does not run with Gradle yet. Note that there was [a PR to solve this](https://github.com/facebook/react-native/pull/10084), but it was not merged.

2. There is no simple way to check if the Native Modules that JavaScript calls are actually included in the native app. With over the air updates, there is no easy way to check if a newer version of JavaScript calls the right method with the correct set of arguments in Native Module.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if a newer version of JavaScript

What do you think about wording this "if a newer version of the JavaScript bundle" so that folks don't confuse it with a newer JavaScript version like ES2030 or whatever 😄


3. Native Modules are always singleton and their lifecycle is typically tied to the lifetime of the bridge. This issue is compounded in brownfield apps where React Native bridge may be started and shut down multiple times.
3. Native Modules are always singleton and their lifecycle is typically tied to the lifetime of the bridge. This issue is compounded in brownfield apps where the React Native bridge may start and shut down multiple times.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/singleton/singletons


4. During the startup process, Native Modules are typically specified in [multiple packages](https://github.com/facebook/react-native/blob/b938cd524a20c239a5d67e4a1150cd19e00e45ba/ReactAndroid/src/main/java/com/facebook/react/CompositeReactPackage.java). We then [iterate](https://github.com/facebook/react-native/blob/407e033b34b6afa0ea96ed72f16cd164d572e911/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java#L1132) over these list [multiple times](https://github.com/facebook/react-native/blob/617e25d9b5cb10cfc6842eca62ff22d39eefcf7b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java#L46) before we finally give the bridge, a [list of Native Modules](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java#L124) to the bridge. This does not need to happen at runtime.
4. During the startup process, Native Modules are typically specified in [multiple packages](https://github.com/facebook/react-native/blob/b938cd524a20c239a5d67e4a1150cd19e00e45ba/ReactAndroid/src/main/java/com/facebook/react/CompositeReactPackage.java). We then [iterate](https://github.com/facebook/react-native/blob/407e033b34b6afa0ea96ed72f16cd164d572e911/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java#L1132) over the list [multiple times](https://github.com/facebook/react-native/blob/617e25d9b5cb10cfc6842eca62ff22d39eefcf7b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java#L46) before we finally give the bridge a [list of Native Modules](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java#L124). This does not need to happen at runtime.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be specific to Android. Is it worth mentioning the process that React Native for iOS uses here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah probably good to have section explaining iOS, though the end goal is to share the c++/jsi layer across platforms


5. The actual methods and constants of a Native Module are [computed during runtime](https://github.com/facebook/react-native/blob/master/ReactCommon/cxxreact/ModuleRegistry.cpp#L81) using [reflection](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java#L75). This can also be done using build time. The individual method calls themselves are sent over a message queue.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also mention here quickness/grayish-area of methods returning result synchronously, I feel we have to "legalize" them.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existing NativeModules system already supports returning values synchronously - not a gray area

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fkgozali Is there an officially supported way to return values from Native Modules synchronously? Or is this just something that's present internally but not exposed?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The support has existed for a while, you may use it, but just note that Chrome debugger doesn't support such synchronous call, so it may make debugging JS harder for you. It's kinda why it's not fully documented. See #7 for some context on debugging.

iOS: Use RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD () instead of RCT_EXPORT_METHOD(): https://github.com/facebook/react-native/blob/ca898f4367083e0943603521a41c48dec403e6c9/React/Base/RCTBridgeModule.h#L175

Android: add (isBlockingSynchronousMethod = true) to your @ReactMethod annotation:
https://github.com/facebook/react-native/blob/c8e000b19a3dc47d58bc3ea1ede8211a370d59bb/ReactAndroid/src/test/java/com/facebook/react/bridge/BaseJavaModuleTest.java#L103

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to note: both companies I have worked at ended up using sync calls in a surgical way for notable user-visible perf improvements.

Expand Down