Commit Graph

4 Commits

Author SHA1 Message Date
Ramanpreet Nara 9b94a541d8 Get CallInvokers from the bridge
Summary:
## Context
For now, assume TurboModules doesn't exist.

**What happens when we call an async NativeModule method?**
Everytime JS calls an async NativeModule method, we don't immediately execute it. The legacy infra pushes the call into some queue managed by `MessageQueue.js`. This queue is "flushed" or "emptied" by the following events:
- **Flushed:** A C++ -> JS call. NativeModule async methods can called with an `onSuccess` and/or `onFail` callback(s). Calling `NativeToJsBridge::invokeCallback` to invoke one of these callbacks is one way for ObjC++/C++/Java to call into JS. Another way is via JSModule method calls, which are initiated by `NativeToJsBridge::callFunction`.
- **Flushed:** When `JSIExecutor::flush` is called. Since TurboModules don't exist, this only happens when we call `JSIExecutor::loadApplicationScript`.
- **Emptied:** When more than 5 ms have passed, and the queue hasn't been flushed/emptied, on the next async NativeModule method call, we add to the queue. Afterwards, we empty it, and invoke all the NativeModule method calls.

**So, what's the difference between flushed and emptied?**
> Note: These are two terms I just made up, but the distinction is important.

If the queue was "flushed", and it contained at least one NativeModule method call, `JsToNativeBridge` dispatches the `onBatchComplete` event. On Android, the UIManager module is the only module that listens to this event. This `onBatchComplete` event doesn't fire if the queue was "emptied".

**Why does any of this matter?**
1. TurboModules exist.
2. We need the TurboModules infra to have `JsToNativeBridge` dispatch `onBatchComplete`, which depends on:
   - **Problem 1:** The queue being flushed on calls into JS from Java/C++/ObjC++.
   - **Problem 2:** There being queued up NativeModule async method calls when the queue is flushed.

In D14656466, fkgozali fixed Problem 1 by making every C++/Java/Obj -> JS call from TurboModules also execute `JSIExecutor::flush()`. This means that, with TurboModules, we flush the NativeModule async method call queue as often as we do without TurboModules. So far, so good. However, we still have one big problem: As we convert more NativeModules to TurboModules, the average size of the queue of NativeModule method calls will become smaller and smaller, because more NativeModule method calls will be TurboModule method calls. This queue will more often be empty than not. Therefore, we'll end up dispatching the `onBatchComplete` event less often with TurboModules enabled. So, somehow, when we're about to flush the NativeModule method call queue, we need `JsToNativeBridge` to understand that we've executed TurboModule method calls in the batch. These calls would have normally been queued, which would have led the queue size to be non-zero. So if, during a batch, some TurboModule async method calls were executed, `JsToNativeBridge` should dispatch `onBatchComplete`.

**So, what does this diff do?**
1. Make `Instance` responsible for creating the JS `CallInvoker`.
2. Make `NativeToJsBridge` responsible for creating the native `CallInvoker`. `Instance` calls into `NativeToJsBridge` to get  the native `CallInvoker`.
3. Hook up `CatalystInstanceImpl`, the Android bridge, with the new JS `CallInvoker`, and the new native `CallInvoker`. This fixes `onBatchComplete` on Android. iOS work is pending.

Changelog:
[Android][Fixed] - Ensure `onBatchComplete` is dispatched correctly with TurboModules

Reviewed By: mdvacca

Differential Revision: D20717931

fbshipit-source-id: bc3ccbd6c135b7f084edbc6ddb4d1e3c0c7e0875
2020-04-01 11:39:18 -07:00
Michel Weststrate 85a4b0f5bd Bump FlipperKit version on iOS to be compatible with react-native-flipper (#28277)
Summary:
This pull request re-applies https://github.com/facebook/react-native/pull/28225, thereby reverting https://github.com/facebook/react-native/commit/4bb17944f18e8ecd20633e49ff143f23210cd976, this time bumping to FlipperKit 0.33.1 which is again compatible with iOS 9 (thanks to https://github.com/facebook/flipper/pull/874)

This is the iOS counterpart of https://github.com/facebook/react-native/pull/28275

## Changelog

[iOS] [Changed] - Upgrade Flipper dependency to 0.33.1
Pull Request resolved: https://github.com/facebook/react-native/pull/28277

Test Plan:
* Bumped the version in our Flipper / RN integration test app first, that runs on 0.62-rc.5, verified everything still works there.
* CI

![Screen Shot 2020-03-10 at 12 12 46](https://user-images.githubusercontent.com/1820292/76311312-94f09f00-62c8-11ea-81db-ced6d16e096b.png)

Reviewed By: mweststrate

Differential Revision: D20369106

Pulled By: TheSavior

fbshipit-source-id: dfeb0446e7f29684a266eca5d0e29ac42cef2ef3
2020-03-10 14:16:55 -07:00
Michel Weststrate fbb94a30bc Bump FlipperKit version on iOS to be compatible with react-native-fli… (#28225)
Summary:
This diff updated FlipperKit to be compatible with the latest react-native-flipper version.

## Changelog

(nothing changes compared to what is already in the changelog for RN 0.62.0)

[iOS][Fixed] - Upgraded FlipperKit to 0.32.2 to ensure compatibility with latest `react-native-flipper`
Pull Request resolved: https://github.com/facebook/react-native/pull/28225

Test Plan:
Tested by upgrading the ReactNativeFlipperExample (https://github.com/facebook/flipper/tree/master/react-native/ReactNativeFlipperExample) to RN 0.62.0-rc.3 and verified the example plugin works. (In D20221558)

![Screen Shot 2020-03-04 at 11 26 52](https://user-images.githubusercontent.com/1820292/75875163-18167e80-5e0b-11ea-8a7d-504bb27757db.png)

Reviewed By: priteshrnandgaonkar

Differential Revision: D20249666

Pulled By: mweststrate

fbshipit-source-id: 71a61c4249f8b767c7767d5c6133436c9bc9cf26
2020-03-04 11:06:47 -08:00
Eloy Durán 4118d79826 Rename autolinking-ios.rb script and bring RNTester and template in line. (#28077)
Summary:
Make the script ready for picking in 0.62.0-rc.3.

* Rename the script to remove ambiguity. “autolinking” is a CLI feature where 3rd-party pods are automatically discovered: https://github.com/react-native-community/cli/blob/master/docs/autolinking.md
* Make the Flipper version of the pods configurable, as requested [here](https://github.com/facebook/react-native/pull/28044#discussion_r378662636).
* Make RNTester’s Podfile use the same form as the template–including enabling Flipper by moving the CPP define into the right place in the Xcode project and out of the Podfile.
* Get rid of Podfile.lock thrashing by making the default CDN spec source explicit.

## Changelog

[iOS] [Changed] - Disambiguate autolinking-ios.rb script from CLI’s “autolinking” feature and bring RNTester & template in line.
Pull Request resolved: https://github.com/facebook/react-native/pull/28077

Test Plan: RNTester builds, as does a new application.

Reviewed By: PeteTheHeat

Differential Revision: D19956392

Pulled By: TheSavior

fbshipit-source-id: 6c800ddb646b49fc45c088978e2794cd7b60ea48
2020-02-19 15:19:26 -08:00