Skip to content
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

Proposal about 0002-Turbo-Modules.md #11

Merged
merged 4 commits into from
Oct 11, 2018

Conversation

axe-fb
Copy link
Collaborator

@axe-fb axe-fb commented Aug 9, 2018

No description provided.

@axe-fb axe-fb changed the title Create 0002-Turbo-Modules.md Proposal about 0002-Turbo-Modules.md Aug 9, 2018
@kelset kelset added the 💡 Proposal This label identifies a proposal label Aug 9, 2018
@kelset
Copy link
Member

kelset commented Aug 9, 2018

I'm a bit confused, but I understand this is a WIP so I guess once it's complete it will be clear.
Just one question:

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

Is this something else from Fabric and from the JSI? Or, like, a proposal that aims to explain a side of the Fabric approach?

Just asking because if it's "Fabric related" I think that adding a new label for Fabric could help.

@axemclion
Copy link

This is not related to fabric, but is built on to of JSI. It could have been done independent of JSI too, but taking advantage of using host objects makes this better.

While fabric is mostly for view managers, this is for native modules.

@kelset
Copy link
Member

kelset commented Aug 9, 2018

So, basically, if I understand correctly:

Native <--> JSI <--> Turbo Module <--> Fabric <--> [code you actually write]

Is this rough scheme correct?

@axe-fb
Copy link
Collaborator Author

axe-fb commented Aug 9, 2018

Not really. While UIManagerModule today is a Native Module, it does not have to be one. We are looking to move FabricUIManager to directly be able to install bindings on JSI. This way, the diagram would be

Java View Managers <--> JNI <--> C++ Shadow Tree <--> JS code => This is Fabric
Java Native Modules <--> JNI <--> C++ <--> JSI <--> JS Code => This is TurboModules

@kelset
Copy link
Member

kelset commented Aug 9, 2018

Ah wow, that's a great diagram - I think it would be helpful to have inside the proposal md :)

@@ -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

Copy link
Collaborator Author

@axe-fb axe-fb left a comment

Choose a reason for hiding this comment

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

Sounds good - if only i could auto merge your changes !!

@grabbou
Copy link
Member

grabbou commented Aug 10, 2018

Interesting - that sounds like what we have talked about two weeks ago at Facebook. I thought that it was part of Fabric when I heard about it - sounds really interesting. Looking forward to the updated proposal that goes a bit more technical into how the native modules work right now.

I am currently planning on working on better link and this is highly interesting to me.

@karanjthakkar
Copy link

@axe-fb Possibly silly question: Since the links in the RFC point to Java code and this diagram also talks about the lifecycle of native modules in Java land, I was wondering if the optimisations mentioned in this RFC also apply to the iOS side of things?

@fkgozali
Copy link

fkgozali commented Aug 10, 2018

I was wondering if the optimisations mentioned in this RFC also apply to the iOS side of things?

It’ll be the same concept in iOS, just without JNI, and it’ll be ObjC classes instead of Java classes. The C++/JSI layer will be shared across platforms.

Copy link

@eliperkins eliperkins left a comment

Choose a reason for hiding this comment

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

Thanks for putting this up here, @axe-fb! I left some general feedback so far.

So far, this discussion seems favored heavily towards the Android side of things... Is it worth creating a separate discussion for how this will work with iOS?


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 😄


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.

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


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.

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

## Background

### JavaScript Interface
The communication between JavaScript and the VM is being standardized using a JavaScript Interface (__JSI__). [Fabric](https://github.com/react-native-community/discussions-and-proposals/issues/4) uses JSI to [expose methods](https://github.com/facebook/react-native/blob/5d9326be29be8688f2238c72c18a9037f983c77d/ReactAndroid/src/main/java/com/facebook/react/fabric/jsc/jni/FabricJSCBinding.cpp#L264) to JavaScript code. Using JSI, JavaScript can hold reference to C++ Host Objects and invoke methods on them.

Choose a reason for hiding this comment

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

s/reference/references

## Detailed design

## Defining Native Modules
Internally, we use flow types to define the Native Modules and their method and signatures (also called "shape of Native Modules"). This is then used to generate Java and ObjC classes with method stubs with matching signatures, which can be extended for the actual implementation. We plan to now generate C++ classes with the same shape, that can then be used to call into the Java/ObjC implementations.

Choose a reason for hiding this comment

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

Does "internally" here refer to Facebook or React Native in general?

Choose a reason for hiding this comment

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

Facebook

Internally, we use flow types to define the Native Modules and their method and signatures (also called "shape of Native Modules"). This is then used to generate Java and ObjC classes with method stubs with matching signatures, which can be extended for the actual implementation. We plan to now generate C++ classes with the same shape, that can then be used to call into the Java/ObjC implementations.

## Initial Processing
All initial processing will be removed as C++ now knows the shape of Native Modules. A proxy similar to the [NativeModuleProxy](https://github.com/facebook/react-native/blob/da7873563bff945086a70306cc25fa4c048bb84b/ReactCommon/cxxreact/JSCExecutor.cpp#L141) will be defined in C++ and [exposed in JavaScript](https://github.com/facebook/react-native/blob/a93e281428f47ba355ba79cda3e98ca7da677636/Libraries/BatchedBridge/NativeModules.js#L152). The initialization can typically happen when the RootView is initialized, but once we are independent of the bridge, this can also happen at other times.

Choose a reason for hiding this comment

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

This will still require a compilation step, however. Is that correct?

Choose a reason for hiding this comment

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

yeah, everything still needs to be compiled, but c++ doesn’t need to ask Java/ObjC about the “shape of the module” anymore, reducing cost of starting up the system

All initial processing will be removed as C++ now knows the shape of Native Modules. A proxy similar to the [NativeModuleProxy](https://github.com/facebook/react-native/blob/da7873563bff945086a70306cc25fa4c048bb84b/ReactCommon/cxxreact/JSCExecutor.cpp#L141) will be defined in C++ and [exposed in JavaScript](https://github.com/facebook/react-native/blob/a93e281428f47ba355ba79cda3e98ca7da677636/Libraries/BatchedBridge/NativeModules.js#L152). The initialization can typically happen when the RootView is initialized, but once we are independent of the bridge, this can also happen at other times.

## Invoking methods on Native Module
When the JavaScript code like `require('NativeModule').DeviceInfoModule` is called, the getter on the proxy is invoked. It would return a reference to object generated in C++ with the shape of the Native Module. Method calls on this reference would simply call over to Java or ObjC. The actual call may also need to have checks to ensure the type and count of arguments.

Choose a reason for hiding this comment

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

Would you mind removing "simply" here? While it may be simple for you, using this word here might make others feel stupid if they don't understand what's so simple about this step.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could use "directly".

Choose a reason for hiding this comment

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

what about modules that are 100% implemented in C++? we have a few modules like this, as did my previous employer. my guess is that it would be an even more efficient communication: JS<->C++<->JS, instead of JS<->C++<->Java/ObjC<->C++<->JS, right?

Choose a reason for hiding this comment

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

to this end, I think a sample AsyncStorage module that uses C++ iostreams to demonstrate the happy and error paths would be useful. specifically, an interface that takes this kind of race/lifecycle issue into account: microsoft/react-native-windows#1888

Choose a reason for hiding this comment

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

C++ only modules are definitely a good use case to cover. We’ll have some hooks after the objc/java hooks are finalized. We do have some C++ based modules internally, but all of them today need to be backed by objc/java, via CxxNativeModule (eg deferring to the os API to get storage path, etc).

For now, first step is to make TurboModule compatible with CxxNativeModule mechanism, then improve the binding to allow direct C++ impl without objc/java as the second step.

Choose a reason for hiding this comment

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

great! avoiding the cost of even "fast" marshaling across JNI/.NET is key for scraping back CPU cycles and memory usage, especially in case of AsyncStorage, WebSocket, etc where giant blobs are potentially being copied today.

## Open Questions
1. How would the C++ object corresponding to the shape of a Native Module know the Java class (for example com.facebook.react.nativemodule.DeviceInfo) that it needs to call. Does this need to be a switch/case or a map in Java/ObjC that is constructed manually ? Would this be done using BUCK dependencies ?
2. Today, native modules usually take `ReactApplicationContext` as a parameter when they are constructed, but they can also be constructed in other ways like using a builder, or multiple parameters in the constructor. How would this be enforced ?
3. How would the C++ code gen happen in Open Source, where BUCK is not used ?

Choose a reason for hiding this comment

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

Perhaps we should look into a "compiler" that is similar to the Relay compiler. That is, some tool that would do the code gen for us in OSS, that BUCK could then depend on.

In the OSS version, we could add this as a build script to projects outside of BUCK.

Choose a reason for hiding this comment

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

yeah, we can treat this as a “script” that the build system calls/executes, e.g. buck calls this script for us internally at Facebook


## 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 under the API section in the [React Native documentation](https://facebook.github.io/react-native/docs/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.

I would explicitly mention here that Native Modules does not directly tight with UI rendering pipeline and that currently-existing View Managers (which are also Native Modules) will go away soon.

Choose a reason for hiding this comment

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

@shergin Is there more information on the coming removal of View Managers?

Choose a reason for hiding this comment

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

view managers will be handled as part of fabric rollout: #4

exact details for migration path is TBD

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.

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.

Internally, we use flow types to define the Native Modules and their method and signatures (also called "shape of Native Modules"). This is then used to generate Java and ObjC classes with method stubs with matching signatures, which can be extended for the actual implementation. We plan to now generate C++ classes with the same shape, that can then be used to call into the Java/ObjC implementations.

## Initial Processing
All initial processing will be removed as C++ now knows the shape of Native Modules. A proxy similar to the [NativeModuleProxy](https://github.com/facebook/react-native/blob/da7873563bff945086a70306cc25fa4c048bb84b/ReactCommon/cxxreact/JSCExecutor.cpp#L141) will be defined in C++ and [exposed in JavaScript](https://github.com/facebook/react-native/blob/a93e281428f47ba355ba79cda3e98ca7da677636/Libraries/BatchedBridge/NativeModules.js#L152). The initialization can typically happen when the RootView is initialized, but once we are independent of the bridge, this can also happen at other times.
Copy link

Choose a reason for hiding this comment

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

I don't think that initialization of a native module should/can be tight with life-cycle of RootView; it can be before or after depending on particular module.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree - all I am saying is that it MAY happen at the rootView initialization for now, but this could also happen at other times - typically during a "bridge-prewarmup"

Choose a reason for hiding this comment

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

keeping a "warm" bridge around initialized with the common first slice of the RAM bundle was used in a few RN Windows apps depending on if people launched the full app, or the "mini" tray app. I suspect this optimization is more common than even I know.

1. How would the C++ object corresponding to the shape of a Native Module know the Java class (for example com.facebook.react.nativemodule.DeviceInfo) that it needs to call. Does this need to be a switch/case or a map in Java/ObjC that is constructed manually ? Would this be done using BUCK dependencies ?
2. Today, native modules usually take `ReactApplicationContext` as a parameter when they are constructed, but they can also be constructed in other ways like using a builder, or multiple parameters in the constructor. How would this be enforced ?
3. How would the C++ code gen happen in Open Source, where BUCK is not used ?
4. Since the generated C++ host objects have the same lifetime of the JS scopes they are referred in, what is the API to tell Java/ObjC when these objects are deallocated ?
Copy link

Choose a reason for hiding this comment

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

Why do they have to have same lifetime as JS scope especially if they are not singletons?

Choose a reason for hiding this comment

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

The C++ host objects have destructors (which will be called when the JS object gets GC'ed) which can tell platform specific classes to "clean up" or deallocate themselves.

Copy link

Choose a reason for hiding this comment

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

Oh, I know, I know. My point is that JS scope usually can increment and decrement ref-counter, this does not always mean that the life-times are identical. I think we are on the same page, I would just clarify the spec.

@dryganets
Copy link

It sounds like we can't use typescript for react-native modules in new architecture unless we will generate/define the flow types for every module.

Is there a way to avoid flow usage?

@acoates-ms
Copy link
Collaborator

I believe the code gen tool has an intermediate data format for defining the module shape. Facebook happens to be generating that from flow, but an additional generator from typescript could easily be written and still use the same code gen. We can probably also add code gen that generates d.ts files also, which would allow the typescript definitions files to always be up to date for native modules -- even ones defined in flow, which would be an improvement over the hand generated ones people currently have to use.

@fkgozali
Copy link

fkgozali commented Sep 9, 2018

Just an update for this exploration:

  • We're still waiting for JSI commit to land in github, so OSS side of RN can start utilizing it. There are a bunch of testing we need to do internally before we're confident to land it.
  • We have some early prototypes for this new system working internally - will be moved to github pending JSI commit.
  • Will share the specifications once it's more stable, and again, pending JSI commit
  • In the first phase:
    • It's all about feature parity with existing implementation for efficiency/perf purpose.
    • Expect all existing NativeModule classes (Java/ObjC) to be 100% backward-compatible with the new system. Eventually, we'll deprecate unnecessary layers.
    • The mechanism to register of each module may need some adjustments, and may involve codegen/scripting, but more details on that later. Eventually:
      • iOS: we're trying to move away from implicit +load inserted by RCT_EXPORT_MODULE() and other macros like RCT_EXPORT_METHOD().
      • Android: we're trying to move away from annotations like @ReactModule and @ReactMethod.

Also, like in #4, feel free to ask questions about the new system and we'll try to answer as best as we can.

@axe-fb
Copy link
Collaborator Author

axe-fb commented Sep 10, 2018

I think we should merge this proposal PR into the repo. Like Fabric, we can use an issue for addressing questions. As we have updates, we can continue to update this doc.

If no one says otherwise, I plan to merge this on Sep 11, 2018, 10 am PDT.

@radex
Copy link
Member

radex commented Sep 10, 2018

This is then used to generate Java and ObjC classes with method stubs with matching signatures, which can be extended for the actual implementation.

I'm not sure I'm understanding this 100% right, so: Are the generated ObjC classes just to make it easier to the user, or does the new system still rely on macros or ObjC runtime somehow to figure out the native module's methods and arguments? (I'm guessing no.)

In other words: will I be able to write plain Swift native modules, without having to hand-write the ObjC .m stub?

@fkgozali
Copy link

Are the generated ObjC classes just to make it easier to the user, or does the new system still rely on macros or ObjC runtime somehow to figure out the native module's methods and arguments?

What's generated is just objc @protocol and Java interface (or abstract class) that your module class just need to implement. The methods in those generated output are based on JS type (e.g. Flow), so you won't need any of the RCT_EXPORT_* macros or @ReactMethod annotations to expose your method signature to JS. This also provides build-time check to make sure that the JS and native "signature" are the same.

For Swift, in theory we could improve the scripting/codegen tool to additionally output Swift protocols.

@radex
Copy link
Member

radex commented Sep 11, 2018

Thanks for clarifying! Unless it’s much more complicated than I expect, I’d be happy to contribute Swift codegen 👍

@axe-fb
Copy link
Collaborator Author

axe-fb commented Sep 28, 2018

I would like to merge this in since we are working on this.

@fkgozali
Copy link

@axe-fb yup let’s do that, then we can open an issue for q&a like fabric

@karanjthakkar
Copy link

Hello 👋 Continuing this discussion from Twitter, I wanted to link to some notes that I wrote that paraphrase, in layman(hopefully) terms, how Turbo Modules is going to work and what are the advantages an RN team can expect out of this: https://gist.github.com/karanjthakkar/b0f75dde2f1717a450878bb9ea917104#turbo-modules. I'd love to know if I have the correct understanding of the implementation and impact from this.

5. ReactModule options like `OnBatchCompleteListener` and `hasEagerInitialization` will be removed. Are they needed anymore, if yes - whats the alternative ?
6. This method allows Native Modules to not be singletons. What would this new pattern look like ? Would this lead to memory leaks ?

## Backward Compatiability
Copy link

Choose a reason for hiding this comment

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

Compatiability/Compatibility/s

@matthargett
Copy link

so... merge?

@axe-fb axe-fb merged commit 6462a12 into react-native-community:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Proposal This label identifies a proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.