Граф коммитов

107 Коммитов

Автор SHA1 Сообщение Дата
James Treanor 6ef91061e8 CocoaPods frameworks compatibility: Step 1 (#25496)
Summary:
This is the first step towards fixing https://github.com/facebook/react-native/issues/25349. These are the changes to the podspec to correctly update dependencies and build config that will cause any breaking change for users or libraries.

I am breaking these changes out from https://github.com/facebook/react-native/pull/25393 as suggested by fkgozali in https://github.com/facebook/react-native/pull/25393#issuecomment-508322884.

These are the changes:

- Made C++ headers in `React-Core` private by default so that ObjC files can import the module without failures.
- Reduced the number of `yoga` headers that are exposed for the same reason as above. As far as I can see this doesn't cause issues but we can find another solution if it does.
- Adding some missing dependencies to fix undefined symbols errors.
- Added `DoubleConversion` to `HEADER_SEARCH_PATHS` where it was missing.

## Changelog

[iOS] [Fixed] - Updated podspecs for improved compatibility with different install types.
Pull Request resolved: https://github.com/facebook/react-native/pull/25496

Test Plan:
Everything should work exactly as before. I have a branch on my [sample project](https://github.com/jtreanor/react-native-cocoapods-frameworks) here which points at this branch to show that it is still working `Podfile` to demonstrate this is fixed.

You can see that it works with these steps:

1. `git clone git@github.com:jtreanor/react-native-cocoapods-frameworks.git`
2. `git checkout podspec-updates`
3. `cd ios && pod install`
4. `cd .. && react-native run-ios`

The sample app will build and run successfully.

Reviewed By: mmmulani

Differential Revision: D16167346

Pulled By: fkgozali

fbshipit-source-id: 1917b2f8779cb172362a457fb3fce686c55056d3
2019-07-10 10:21:38 -07:00
Kevin Gozali 01a42bc368 TM iOS: Prevent module cache invalidation race
Summary: There are still race condition during bridge invalidation. Some modules may be accessing other modules during invalidation, but it's racing with the TM manager clearing the cache.

Reviewed By: JoshuaGross

Differential Revision: D15947488

fbshipit-source-id: 3bd51382264f538a03ca565b0f099da40c3daadf
2019-06-21 15:54:01 -07:00
zhongwuzw af59323d81 TM: Backward compatibility of calling invalidate on module's method queue (#25264)
Summary:
Backward compatibility for `invalidate` method, because for the old module system, we ensure call all methods of modules on `methodQueue`, we also need to keep this rule to avoid some race condition in module.

## Changelog

[iOS] [Fixed] - Backward compatibility of calling invalidate on module's method queue
Pull Request resolved: https://github.com/facebook/react-native/pull/25264

Test Plan: Ensure `invalidate` method of `module` be called on `methodQueue`.

Reviewed By: PeteTheHeat, JoshuaGross

Differential Revision: D15944532

Pulled By: fkgozali

fbshipit-source-id: e260792bc6b86a48cdf376282063cbabccbf26f0
2019-06-21 15:54:01 -07:00
Kevin Gozali 72e276a0f0 TM iOS: Keep LongLivedObject longer past invalidation
Summary:
There can be a race condition between bridge invalidation (hence TM binding invalidation) with pending Promise reject/resolve invocation. If invalidation happens first, invoking the resolve/reject from ObjC class might end up accessing destroyed PromiseWrapper, causing hard crash randomly.

The proper fix is to switch the objc promise resolve/reject block (objc block) to use C++ PromiseWrapper directly, such that the lifecycle of the shared_ptr<> can be correct.

Reviewed By: RSNara

Differential Revision: D15801403

fbshipit-source-id: 9b0c7d323b18d94e920ea3eafc3a6916dadba247
2019-06-13 09:13:51 -07:00
Ramanpreet Nara e908f7a3fe Fix TurboCxxModules::get
Summary: Make `TurboCxxModule::get` return `undefined` when trying to access methods that don't exist. This is what `TurboModule::get` does. So, it makes sense that this behaviour is preserved in TurboCxxModule.

Reviewed By: mdvacca

Differential Revision: D15780044

fbshipit-source-id: 13aeae081655735ef634f1dc09c0dc70a3a3a893
2019-06-13 09:07:24 -07:00
Kevin Gozali c0bf53b716 TM iOS: attempt to convert number args using RCTConvert as well
Summary: Some native modules methods expects number-based args like `NSDate`. For backward compatibility, the incoming numbers should be converted using RCTConvert, just like object args.

Reviewed By: mdvacca

Differential Revision: D15748968

fbshipit-source-id: 4db2cb0c41eda1bbe8cde7b0365d9c3d675f5fb5
2019-06-10 18:41:22 -07:00
Kevin Gozali 7225daf98d TM iOS: Support @synthesize methodQueue auto queue creation
Summary: Some native modules defined `synthesize methodQueue` which will ask the bridge to create a new serial queue for them. TM system needs to preserve this behavior for backward compatibility. In the future though, this magic shall be removed.

Reviewed By: mdvacca

Differential Revision: D15748198

fbshipit-source-id: 66a4b60a2769ac967a8d3bb00c4c635a68daebbc
2019-06-10 18:41:22 -07:00
Kevin Gozali 360e999937 TM iOS: reduce the scope of cache access lock
Summary: We just need to protect access to the cache, we don't need to protect the entire module lookup, because a module initialization may try to lookup another module, causing deadlocks.

Reviewed By: RSNara

Differential Revision: D15690645

fbshipit-source-id: cbb780db8699a94f2c9a2e121b35ddad2b125b65
2019-06-06 09:40:21 -07:00
Ramanpreet Nara a44ab2957c Protect access to RCTTurboModuleCache
Summary: The `_rctTurboModuleCache` `std::unordered_map` can be accessed by multiple threads at the same time via the `provideRCTTurboModule` method. Since `provideRCTTurboModule` both reads and writes to `_rctTurboModuleCache`, this is really bad because we could end up reading from `_rctTurboModuleCache` while it's in an invalid state. Therefore, in this diff, I'm making it so that only one thread at a time can enter `provideRCTTurboModule`.

Reviewed By: fkgozali

Differential Revision: D15609987

fbshipit-source-id: e24e1f5cc2351d8cbb820b7a97074aacd06eec9d
2019-06-03 16:16:09 -07:00
Ramanpreet Nara e1102b43ff Implement Android Cxx TurboModule support
Summary:
## Summary
This diff does a bunch of things:
1. The TurboModule resolution algorithm on Android now supports C++ TurboModules.
2. `SampleTurboCxxModule` is moved from `ReactCommon/turbomodule/samples/platform/ios/` to `ReactCommon/turbomodule/samples` so that both iOS and Android can share it.
3. `CatalystTurboModuleManagerDelegate::getTurboModule(std::string, JSCallInvoker)` now understands and returns `SampleTurboCxxModule`.

Reviewed By: mdvacca

Differential Revision: D15253477

fbshipit-source-id: 3def91911b091f8cf93be17decd245a0499ed718
2019-05-22 13:16:13 -07:00
zhongwuzw a352ecfabd TM iOS: Remove retainArguments && do retain by us explicitly (#24849)
Summary:
This is a TODO, we met crash if we don't call `retainArguments` when return type is like `NSDictionary`, the reason is `getReturnValue` don't retain the return value, so we need to using `__bridge` to transfer ownership to OC type.
Also add `resolveBlock` and `rejectBlock` to `retainedObjectsForInvocation`.

cc. cpojer

## Changelog

[iOS] [Fixed] - Remove retainArguments && do retain by us explicitly
Pull Request resolved: https://github.com/facebook/react-native/pull/24849

Differential Revision: D15369209

Pulled By: fkgozali

fbshipit-source-id: 8431d03705d8476f38c8b5d29630489a545d373a
2019-05-15 21:50:40 -07:00
Kevin Gozali 8662d9d3b0 TM iOS: Added backward-compatible ObjC invalidation logic
Summary:
Some ObjC NativeModules conform to `RCTInvalidating` protocol and implements `invalidate` method. This is typically used to clean things up during bridge teardown or reload. In TurboModule system the invalidate method can be replaced by pure destructors, but for backward compatibility, we call existing invalidate method on each module during teardown.

This also cleans up all existing LongLivedObject's.

Reviewed By: mdvacca, RSNara

Differential Revision: D15365655

fbshipit-source-id: 802844b39b5b7adb54970ea541f4d744bbf9e480
2019-05-15 17:36:05 -07:00
RCiesielczuk cd2f8c567b Introduce TurboModule Setup Metric (#24732)
Summary:
With the introduction of TurboModules, it would be beneficial to measure the setup time of these modules, as we currently have it in place for NativeModules.

The instantiation of the TMs occurs in the `RCTTurboModuleManager`. In order to successfully measure the time it took to setup the module, we need to ensure that we don't take into account cached modules. As such, we need to:

1. Check if module is in `_turboModuleCache`
  a. Start mark for `RCTPLTurboModuleSetup` tag if not found
2. Get the TM via `[self provideTurboModule:]`
3. Check if module is in `_turboModuleCache`
   a. Stop mark for `RCTPLTurboModuleSetup` if we did not find module in cache prior to **step 2** and if it's now present in the cache.
   b. Notify about setup time if the above is true.
4. Return TM

## Changelog

[iOS] [Added] - Gain insights on the the turbo module setup times by observing `RCTDidSetupModuleNotification`. The userInfo dictionary will contain the module name and setup time in milliseconds. These values can be extracted via `RCTDidSetupModuleNotificationModuleNameKey` and `RCTDidSetupModuleNotificationSetupTimeKey`.
Pull Request resolved: https://github.com/facebook/react-native/pull/24732

Differential Revision: D15362088

Pulled By: RSNara

fbshipit-source-id: e6a8044e4aba5a12ae63e9c7dbf707a17ec00180
2019-05-15 15:28:10 -07:00
zhongwuzw 36f9a4aca7 TM iOS: Consolidate RCTDidInitializeModuleNotification sender to batchedBridge (#24866)
Summary:
Consolidate sender of notification to `batchedBridge` for TM and old module system, old module implementation can see dc893756b8/React/Base/RCTModuleData.mm (L191-L193).

## Changelog

[iOS] [Fixed] - Conlidate RCTDidInitializeModuleNotification sender to batchedBridge
Pull Request resolved: https://github.com/facebook/react-native/pull/24866

Differential Revision: D15357566

Pulled By: RSNara

fbshipit-source-id: 479575c1e9781d8aea9fa3aea9e0283d24df7e81
2019-05-15 12:31:30 -07:00
Emily Janzer c75f062a77 Change JSCallInvoker to be an abstract class
Summary: The existing implementation of JSCallInvoker holds a reference to Instance (aka the bridge). This diff makes JSCallInvoker an abstract base class that's extended by BridgeJSCallInvoker, which is what's returned by CatalystInstance to initialize TurboModules. This will allow us to add another class that derives from JSCallInvoker that doesn't rely on the bridge.

Reviewed By: shergin

Differential Revision: D15252375

fbshipit-source-id: 75eee2ca149235a63e2a2cd8cdd361b163d1d1ab
2019-05-13 12:04:33 -07:00
Kevin Gozali 0436f81595 TM iOS: Fixed mixed-up module lookup from native side
Summary:
There are 2 issues:
* RCTTurboModuleManager may be initialized too late, so the module lookup from native via `[bridge moduleForClass:]` may end up going to the old system incorrectly
* In the case where JS is asking for a nativemodule, we may be incorrectly registering modules that are marked as RCTTurboModule - they should be ignored instead

Reviewed By: mmmulani, RSNara

Differential Revision: D15247632

fbshipit-source-id: 4e0fae33923810c74966b38276b206ac00723012
2019-05-07 19:29:16 -07:00
Ramanpreet Nara 913ff9f5ae Fix OSS build
Summary: iOS TurboModules work in OSS. I pulled out JSCallInvoker from `React-turbomodule-core`, as a part of D15055511. In this diff, I'm adding it back in.

Reviewed By: mdvacca

Differential Revision: D15195007

fbshipit-source-id: 64ee294a68c0a63ba400b8a829864c6619d3311a
2019-05-03 13:31:21 -07:00
Ramanpreet Nara ef4955fefe Make async calls work
Summary:
JSCallInvoker requires a `std::weak_ptr<Instance>` to create. In our C++, `CatalystInstance` is responsible for creating this `Instance` object. This `CatalystInstance` C++ initialization is separate from the `TurboModuleManager` C++ initialization. Therefore, in this diff, I made `CatalystInstance` responsible for creating the `JSCallInvoker`. It then exposes the `JSCallInvoker` using a hybrid class called `JSCallInvokerHolder`, which contains a `std::shared_ptr<JSCallInvoker>` member variable. Using `CatalystInstance.getJSCallInvokerHolder()` in TurboModuleManager.java, we get a handle to this hybrid container. Then, we pass it this hybrid object to `TurboModuleManager::initHybrid`, which retrieves the `std::shared_ptr<JSCallInvoker>` from the `JavaJSCallInvokerHandler`.

There were a few cyclic dependencies, so I had to break down the buck targets:
- `CatalystInstanceImpl.java` depends on `JSCallInvokerHolderImpl.java`, and `TurboModuleManager.java` depends on classes that are packaged with `CatalystInstanceImpl.java`. So, I had to put `JSCallInvokerHolderImpl.java` in its own buck target.
- `CatalystInstance.cpp` depends on `JavaJSCallInvokerHolder.cpp`, and `TurboModuleManager.cpp` depends on classes that are build with `CatalystInstance.cpp`. So, I had to put `JavaJSCallInvokerHolder.cpp` in its own buck target. To make things simpler, I also moved `JSCallInvoker.{cpp,h}` files into the same buck target as `JavaJSCallInvokerHolder.{cpp,h}`.

I think these steps should be enough to create the TurboModuleManager without needing a bridge:
1. Make `JSCallInvoker` an abstract base class.
2. On Android, create another derived class of `JSCallInvoker` that doesn't depend on Instance.
3. Create `JavaJSCallInvokerHolder` using an instance of this new class somewhere in C++.
4. Pass this instance of `JavaJSCallInvokerHolder` to Java and use it to create/instatiate `TurboModuleManager`.

Regarding steps 1 and 2, we can also make JSCallInvoker accept a lambda.

Reviewed By: mdvacca

Differential Revision: D15055511

fbshipit-source-id: 0ad72a86599819ec35d421dbee7e140959a26ab6
2019-05-03 13:31:20 -07:00
Ramanpreet Nara 7a8de7db46 Replace throwIfJNIReportsPendingException
Summary: Looks like FBJNI exports a C Macro that does exactly what `throwIfJNIReportsPendingException` does. Therefore, I'm replacing `throwIfJNIReportsPendingException` with calls to `FACEBOOK_JNI_THROW_PENDING_EXCEPTION()`.

Reviewed By: mdvacca

Differential Revision: D15174820

fbshipit-source-id: 9dfb519352cbd5f37527675323cbabad05e31d4a
2019-05-03 12:00:19 -07:00
Ramanpreet Nara a9650709e3 Fix multiple invocations of getConstants
Summary:
`jclass` in `JNI` is just a regular local reference. Therefore, it's unsafe to keep a static reference to it. Link: http://journals.ecs.soton.ac.uk/java/tutorial/native1.1/implementing/refs.html.

This bug made it so that when you clicked on `getConstants` twice in the TurboModule playground, the app would crash.

Reviewed By: mdvacca

Differential Revision: D15174821

fbshipit-source-id: 13b2b8726473acc9b07306558044d26bed0db92d
2019-05-03 12:00:19 -07:00
Ramanpreet Nara c9006ce5fb Refactor getConstants implementation
Summary: Previously, we'd override the `TurboModule::get` method inside the `JavaTurboModule` class to return a special `jsi::Function` in the case that the property being accessed was "getConstants". We really don't need to do this because we can simply special-case the invocation of the `getConstants` method inside the `JavaTurboModule::invokeJavaMethod` method.

Reviewed By: mdvacca

Differential Revision: D15174822

fbshipit-source-id: 0ee705be841757d3870c908da911c3872b977a9f
2019-05-03 12:00:18 -07:00
Kevin Gozali 0708e28caf TM: removed unused virtual invokeMethod() and invalidate()
Summary:
* invokeMethod() ends up not useful because each platform has its own way of invoking the platform methods
* invalidate() is not necessary because there's already the destructor of each C++ class

Reviewed By: mdvacca

Differential Revision: D15187833

fbshipit-source-id: 9478ed1e6288da30c67179e03a7bc7da6043280b
2019-05-02 14:12:34 -07:00
Kevin Gozali 694bf7104c TM iOS optimization: codegen @selector for each method call.
Summary: This adds support for specifying the exact selector name for each exposed ObjC method. This allows us to avoid dynamic method lookup every time a method is called from JS.

Reviewed By: RSNara

Differential Revision: D15141611

fbshipit-source-id: ed2820782ab013369e4e1f22dbce31d9838a17bb
2019-04-30 13:58:35 -07:00
Ramanpreet Nara e7a8b26f22 Fix ObjCTurboModule::getArgumentTypeName
Summary: In `ObjCTurboModule::getArgumentTypeName`, I replaced all instances of `':'` with `''` to transform the selector into a TurboModule methodName. This transformation works when the method has 0 or 1 argument, however, it breaks when the method has more than 1 argument. In all cases, we just want to get the substring until the first `':'`.

Reviewed By: fkgozali

Differential Revision: D15056937

fbshipit-source-id: 3a7dce1ce62ca9758e46c0af951b269166d68454
2019-04-23 17:52:21 -07:00
Ramanpreet Nara c9c8f8d5b3 Remove jClassName_ member
Summary: `jClassName_` is unnecessary because you can use `JNIEnv::GetObjectClass` to get the TurboModule's Java class.

Reviewed By: fkgozali

Differential Revision: D14937480

fbshipit-source-id: 2c1c9be53217331152270dbac3d13f372a2ed818
2019-04-22 15:33:06 -07:00
Kevin Gozali 1149d85b9f iOS RNTester: install SampleTurboModule regardless
Summary: This installs the sample module to the RNTester.xcodeproj without using any TurboModule infra. This is possible because SampleTurboModule is backward compatible with the existing NativeModules system. This also fixes CI test failure: https://circleci.com/gh/facebook/react-native/84752

Reviewed By: RSNara

Differential Revision: D14987572

fbshipit-source-id: f5f2c4330c7f6558c7d4beeb43198869090dee02
2019-04-17 16:37:49 -07:00
Ramanpreet Nara 129aeacf48 Add exception handling to method invocations
Summary:
If you call into a Java method (from C++ using JNI) that raises an exception, the JNI call won't actually raise a C++ error. Instead, the `JNIEnv` will record the pending Java exception and the C++ will continue executing as normal. This is bad because the next time you call into JNI, the app will actually crash, unless you explicitly cleared the exception using `JNIEnv::ExceptionClear()` before the JNI call.

With respect to TurboModules, we need to make sure that RedBoxes show up whenever a native methods raise an exception. We also don't want the app to crash when a JNI method call fails because of a raised exception. Therefore, in this diff, I raise a C++ exception if `JNIEnv::ExceptionCheck()` is true.

Reviewed By: mdvacca

Differential Revision: D14738540

fbshipit-source-id: 4c3063aa93ae7aef025bd2dab6b45059bb8fb409
2019-04-16 09:18:14 -07:00
Ramanpreet Nara d1c35aaa84 Add support for Promises
Summary:
If the return type of a TurboModule method is `Promise`, the infra should create a `com.facebook.react.bridge.Promise` object and pass it as the final argument of the TurboModule Java method call. The Java TurboModule method can then do some work asynchronously and either resolve or reject the promise at some point in time.

**Note:** I stacked a diff for error handling on top of this one.

Reviewed By: mdvacca

Differential Revision: D13653156

fbshipit-source-id: 4c30c3223ad8f47c6ba7f1236527aaced01c8ae8
2019-04-16 09:18:14 -07:00
Kevin Gozali 4da6e4a042 TM iOS: Install SampleTurboModule in RNTester (pods)
Summary: This sets up RCTSampleTurboModule (and other variants) in RNTester when built with cocoapods. There's no call site yet though. And RNTester.xcodeproj doesn't support it.

Reviewed By: cpojer

Differential Revision: D14932535

fbshipit-source-id: db8eafd6777cbec8f3592dafdccbdd7cf44e38bc
2019-04-15 12:25:48 -07:00
Kevin Gozali a2aba45067 TM iOS: Move SampleTurboModule to OSS
Summary:
This provides various versions of SampleTurboModule, that are:
* compatible with existing NativeModule
* TurboModule compliant

Variants:
* RCTSampleTurboModule (traditional objc module)
* RCTSampleTurboCxxModule (objc++ module using CxxModule)
* SampleTurboModule (pure C++ impl of a TurboModule, no ObjC)

As noted in some files, they need to be codegen'ed based on the `NativeSampleTurboModule.js` (Flow type). The codegen script is not yet usable in OSS (we'll work on it some time in H2 2019). For now, these files need to be manually synced with Flow type.

Reviewed By: cpojer

Differential Revision: D14932539

fbshipit-source-id: fb887192384e5e6e4dff4cac68b4e037a4783cd9
2019-04-15 12:25:47 -07:00
Kevin Gozali e612b9b0f5 TM iOS: Install TurboModule system in RNTester (pod) by default
Summary:
For CocoaPods variant only: install TurboModule binding so that sample modules can start using it. This commit only installs `global.__turboModuleProxy` - no sample module is provided.

Note: RNTester.xcodeproj will NOT have TurboModule enabled, due to complication in the .xcodeproj setup (doable, but maybe for some other time...)

To test:

```
console.error(global.__turboModuleProxy == null ? 'BOO' : 'YAY!');
```

Saw `YAY!` in RNTester pod version.

Reviewed By: cpojer

Differential Revision: D14932536

fbshipit-source-id: 3dc083da9154ec320ce6789ec7f2cef5a08fd6a7
2019-04-15 12:25:47 -07:00
Kevin Gozali 71a8944b39 TM iOS: Use weak_ptr to pass around Instance to avoid unreleased refs
Summary:
There is a timing issue when reloading the bridge (in dev mode) and the tear down of the TurboModules. This causes `Instance` to never get freed, hence the "bridge" isn't cleaning up properly. The side effect can be bogus error saying that it's unable to find a module.

To address this, JSCallInvoker should just take in weak_ptr<Instance>.

Reviewed By: RSNara

Differential Revision: D14739181

fbshipit-source-id: f9f2a55486debaeb28d3d293df3cf1d3f6b9a031
2019-04-02 18:11:18 -07:00
Ramanpreet Nara 46944b7937 Implement auto-conversions from primitives to Objects
Summary:
If a NativeModule method requires an optional boolean argument, our codegen translates those optional booleans into `NSNumber*` instead of `BOOL`. The reason why is probably because this is the closest object analogue to `BOOL` in Objective C. The same boxing occurs with numbers. If the type of a number argument in JavaScript is optional, we'll map it to the `NSNumber*` Objective C type instead of `double`. Our existing TurboModules argument conversion code would not take this behaviour into account. Therefore, we'd try to insert a `BOOL` where the `NSInvocation` would expect a `NSNumber*`. This, in turn, would cause the app to crash. (Why would it crash at the point of NSInvocation retainArguments, I'm still not sure).

Our flow typechecking ensures that if the type of a method argument is a boolean, we pass in a boolean. Therefore, on the Native side, if we detect a boolean, we can check the type of the Native argument to see whether we should box the primitive. If the native argument type is an object, then we know it has to be an `NSNumber*` in both cases, so we simply wrap the `BOOL` or `double` in a `NSNumber*`.

Reviewed By: shergin

Differential Revision: D14679590

fbshipit-source-id: c394a878492aab8e98c71d74ec8740a94fc3a6c5
2019-03-31 02:49:35 -07:00
Kevin Gozali a43e666a34 TM iOS: force flush message queue when calling into JS from native
Summary: When calling into JS (e.g. promise resolve/reject, callback) in TurboModule, we bypass the bridge's message queue. At times this causes race condition, where there are a bunch of pending UI operations (in RCTUImanager) waiting to be flushed, but nothing adds calls to the message queue. Usually tapping the screen will trigger the flush because we're sending down touch events to JS.

Reviewed By: JoshuaGross

Differential Revision: D14656466

fbshipit-source-id: cb3a174e97542bf80f0a37b4170b6a8e6780fa35
2019-03-29 01:39:39 -07:00
Ramanpreet Nara 58cd20459f Force JS null to be translated to ObjC nil
Summary:
In D14571128, we made it so that when a JS object's property was `undefined`, we wouldn't insert that property into the corresponding NSDictionary. Here are two important observations about that diff:
1. ALL JS `null`s were now being converted to `NSNull`, and JS `undefined`s were now being converted to `nil`.
2. If a JS object's property was explicitly `null`, then we'd insert `NSNull` into the corresponding dictionary.

Considering that when a property doesn't exist in a `NSDictionary`, property access returns `nil`, I've made it so that if a JS object's property is either `null` or `undefined`, then we simply do not insert it in the corresponding `NSDictionary`. Also, I've reverted #1 and made it so that `undefined` and `null` always map to the ObjC `nil`.

This shouldn't unfix the problem that D14571128 was trying to fix.

Here's my understanding of the problem that D14571128 was trying to fix (to make sure I'm not breaking something by this diff).

This method was invoked from JS.
```
RCT_EXPORT_METHOD(logEvents:(NSDictionary *)events)
{
  RCTAssert(events, @"You should provide events for logger");
  [[NSNotificationCenter defaultCenter] postNotificationName:@"FBReactPerfLoggerDidReceiveEventsNotification"
                                                      object:nil
                                                    userInfo:@{@"FBReactPerfLoggerUserInfoPerfEventsKey" : [events copy]}];
}
```

The above dispatch calls into this method, which appends `events` into `_pendingJSPerfEvents`.

```
- (void)reactPerfLoggerDidReceiveEvents:(NSNotification *)notification
{
  NSDictionary *events = notification.userInfo[@"FBReactPerfLoggerUserInfoPerfEventsKey"];
  if (events) {
    dispatch_async(_eventQueue, ^{
      if (self->_sessionData.perfLoggerFlagId != nil) {
        if ([self processJSPerfEvents:events]) {
          [self reportMetricsIfFinished];
        }
      } else {
        [self->_pendingJSPerfEvents addObject:events];
      }
    });
  }
}
```

Then, in `_processJSPerfEvents`, we do the following (link: https://fburl.com/tr4wr2a7):
```
NSNumber *actionId = events[@"actionId"];
if (actionId) {
  self->_sessionData.actionId = actionId;
}
```

So, if `undefined` or `null` was passed in as the `actionId` property of the `events` JS object in `FBReactPerformanceLogger logEvents:`, then we'd default the `NSDictionary` to have `NSNull` in the corresponding property. This is bad because we had this line in FBReactWildePerfLogger (link: https://fburl.com/2nsywl2n):  `actionId ? [actionId shortValue] : PerfLoggerActions.SUCCESS`. Essentially, this is the same problem that my diff is trying to fix.

Reviewed By: fkgozali

Differential Revision: D14625287

fbshipit-source-id: c701d4b6172484cee62494256175e8b205b23c73
2019-03-26 14:28:23 -07:00
Ramanpreet Nara 8618a5824f Add support for argument conversion via RCTConvert
Summary:
With our current infra, we support automatic conversion of method arguments using `RCTConvert`.

```
RCT_EXPORT_METHOD(foo:(RCTSound*) sound)
{
  //...
}
```

```
interface RCTConvert (RCTSound)
+ (RCTSound *) RCTSound: (NSDictionary *) dict;
end

implementation RCTConvert (RCTSound)
+ (RCTSound *) RCTSound: (NSDictionary *) dict
{
  //...
}
end
```

```
export interface Spec extends TurboModule {
  +foo: (dict: Object) => void,
}
```

With this setup, when we call the foo method on the TurboModule in JS, we'd first convert `dict` from a JS Object to an `NSDictionary`. Then, because the `foo` method has an argument of type`RCTSound*`, and because `RCTConvert` has a method called `RCTSound`, before we invoke the `foo` NativeModule native method, we first convert the `NSDictionary` to `RCTSound` using `[RCTConvert RCTSound:obj]`. Essentially, if an argument type of a TurboModule method is neither a primitive type nor a struct (i.e: is an identifier), and it corresponds to a selector on `RCTConvert`, we call `[RCTConvert argumentType:obj]` to convert `obj` to the type `argumentType` before passing in `obj` as an argument to the NativeModule method call.

**Note:** I originally planned on using `NSMethodSignature` to get the argument types. Unfortunately, while the Objective C Runtime lets us know that the type is an identifier, it doesn't inform us which identifier it is. In other words, at runtime, we can't determine whether identifier represents `RCTSound *` or some other Objective C class. I figure this also the reason why the old code relies on the `RCT_EXPORT_METHOD` macros to implement this very same feature: https://git.io/fjJsC. It uses `NSMethodSignature` to switch on the argument type, and then uses the `RCTMethodInfo` struct to parse the argument type name, from which it constructs the RCTConvert selector.

One caveat of the current solution is that it won't work work unless we decorate our TurboModule methods with `RCT_EXPORT_METHOD`.

Reviewed By: fkgozali

Differential Revision: D14582661

fbshipit-source-id: 3c7dfb2059f031dba7495f12cbdf406b14f0b5b4
2019-03-22 16:23:40 -07:00
Ramanpreet Nara ef512194a6 Implement struct arguments for methods
Summary:
Before invoking TurboModule ObjC methods, we loop through all the arguments and transform them from `jsi::Value` to ObjC data structures. `jsi::Value`s that represent JS Objects get converted to `NSDictionary`s in ObjC. This isn't good enough because `NSDictionary` isn't typed. What we really need is a C/C++ struct that represents the type of the JS Object.

Therefore, for every argument of a TurboModule method that is a JS Object, this diff allows you to specify a method on `RCTCxxConvert` (which you have to declare and implement) that transforms that type of JS Object into something else. Basically, with the changes in this diff, we'll be able to transform JS Objects into C++ struct instances, which gives us type safety for JS Object method argumetns to TurboModule functions.

I modified the codegen to also create a mapping from NativeModule method name => argument num => `RCTCxxModule` conversion selector. This way, the FB codegen that generates the `RCTCxxConversion` function also informs our TurboModule system which conversion function to use before we call the method requiring complex argument. I just had to extend the `ObjCTurboModule` class to accomplish this.

The old system relies on the additional method generated by `RCT_EXPORT_METHOD` macro. It takes the written code, does string processing on it to parse the type of the struct arguments, and then replaces all instances of `::` with `_`. Super hacky. I didn't take this approach because it seemed unnecessarily hacky brittle.

The other approach I considered was to try to use reflection to infer the type of the struct at runtime. We'd have to do a bit of string processing and concat the TurboModule class name with the struct type. The solution would be simpler because it'd only modify the objective C and it wouldn't touch the Codegen system. **Edit:** I implemented it here: D14513078. The one downside of this design is that the name of the conversion function is individually constructed in two different locations in the code (i.e: no source of truth for this name). The other downside is that we have to rely on `RCTBridgeModuleNameForClass`, which we want to eliminate. This also computationally more expensive since it requires string processing/regex parsing and additional reflection. Therefore, we decided to move on with the current solution.

Reviewed By: fkgozali

Differential Revision: D14513429

fbshipit-source-id: 3d1b87e02ee908a19305686ff82b2ed624d8ac67
2019-03-21 17:37:18 -07:00
Kevin Gozali 3e8d2a18d7 iOS: undefined prop in an object shouldn't become [NSNull null] in the dictionary
Summary: [iOS] [Fixed] - The existing logic defaults `undefined` & `null` in JS to be `[NSNull null]` when converting JS object to `NSDictionary`. Let's not insert the prop to the dictionary if it's `undefined`.

Reviewed By: blairvanderhoof

Differential Revision: D14571128

fbshipit-source-id: e03c713b055672b0a001d3305d694912ee36ab36
2019-03-21 17:05:57 -07:00
ericlewis 80ccbdcfa0 TurboModules: add podspec (#23927)
Summary:
Add's a podspec to allow linking / building turbomodules.

[iOS] [Added] - podspec for turbomodules
Pull Request resolved: https://github.com/facebook/react-native/pull/23927

Reviewed By: mdvacca, RSNara

Differential Revision: D14474598

Pulled By: fkgozali

fbshipit-source-id: afafce0f0af31469a3ef91616575b3b0f36055bc
2019-03-15 11:44:27 -07:00
Ramanpreet Nara 8ad695b10e Broadcast TurboModule creation
Summary:
When TurboModules are created, we need to broadcast this event using `NSNotificationCenter`. This is important because a number of TurboModules/ObjC classes listen to this notification and perform side effects.

I found these using https://fburl.com/codesearch/j0a9p3dr.
- FBReactKitServerSnapshotTests: After the RCTExceptionsManager is initialized, it assigns itself as the delegate to the RCTExceptionsManager instance.
- FBProfilePictureUploadFlowController: After FBProfileFrameNativeModule is initialized, it calls `[module setFlowController: self]`
- RCTFBSession: After every NativeModule is initialized, if that NativeModule conforms to the RCTFBSessionModule protocol, assign the session to it.
- TwilightLocalMedia: Hack to figure out when the bridge is initialized.
- FBMarketplaceMenuItemHandler: After FBMarketplaceNativeModule is initialized, attach a data source obtained from the session to the module. Then, attach module to data source.
- RCTModuleInitTests: Set local state based on if certain modules are initialized (https://fburl.com/e34kezb5).
- IGReactModule: When the RCTImageLoader is initialized, set it's image cache to `[IGReactImageCache new]`.
- RCTIGUserSession: Like RCTFBSession
- FBReactModuleTracker: This keeps a list of all dispatched notifications (for some reason).
- RCTModuleInitNotificationRaceTests: Sets self.didDetectViewManagerInit to true if RCTTestViewManager is initialized.

Reviewed By: fkgozali

Differential Revision: D14281557

fbshipit-source-id: 9fcf1bb5f5650fcc3a3e50c6d93405b0b618e271
2019-03-01 12:29:34 -08:00
Kevin Gozali 29613a0364 TM iOS: convert method invocation result in JS thread instead of in the method queue
Summary: [IOS] [FIXED] - This may cause weird race condition if JSI is accessed via multiple threads within the same method invocation. Since only the objc class methods need to be executed on the right queue, we can keep the conversion to jsi::Value in the originating JS thread.

Reviewed By: RSNara

Differential Revision: D14277167

fbshipit-source-id: c506aebb71e190e2afcbf19dce05088ce2b97833
2019-03-01 11:05:00 -08:00
Kevin Gozali 45288a0f63 TM iOS C++: don't throw if a method is not found
Summary: Let JS decide if a missing method should be treated as an error, or whether it allows optional methods (e.g. methods that are only for android or for ios).

Reviewed By: JoshuaGross

Differential Revision: D14155799

fbshipit-source-id: 1e298b46a59761cf09e98147da885b1e9a9a675a
2019-02-20 14:55:08 -08:00
Kevin Gozali a1b3421d10 TM iOS: allow fallback __turbo__* methods to proxy NSDictionary types to stricter types
Summary:
Some modules may have args with types stricter than just `NSDictionary`. For now, allow these modules to manually define a __turbo__* variant of the same method that receives basic NSDictionary/NSArray typed args. That helper method can do the conversion to the stricter type as necessary. This is only needed during migration phase of TurboModule.

Without this workaround, existing methods may get unrecognized selector errors.

Reviewed By: PeteTheHeat

Differential Revision: D14115981

fbshipit-source-id: ca7fcf497490ef9cce14c3d3444991c50d3cb550
2019-02-15 20:06:15 -08:00
Jonathan Kim c02f278d8f xplat// -> fbsource//xplat/ for xplat/js
Reviewed By: scottrice

Differential Revision: D14076351

fbshipit-source-id: f803d2af438a5eb3f18c981793e17b6293d8ef1c
2019-02-14 23:53:32 -08:00
Kevin Gozali 344b32bb64 TM iOS: avoid bad memory access when passing prop name to a lambda
Summary: Depending on the timing of the method call from JS to a CxxModule, we may be accessing memory that has been deallocated, causing exception to RN runtime. This fixes it.

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D14033831

fbshipit-source-id: 5a77aa41223b1fc3146dcf78b7f8e93375605d6d
2019-02-11 17:29:14 -08:00
Kevin Gozali d9f34bdbb2 TM iOS: added helper to check whether a class is TurboModule compatible
Summary: Similar macro as the existing one, but this one checks for the class directly.

Reviewed By: RSNara

Differential Revision: D14016664

fbshipit-source-id: aae9a9c1cc95f56d2eff6c9021a714ed4a843db3
2019-02-08 21:08:51 -08:00
Kevin Gozali 94925d5eb7 TM iOS: Verify that module class conforms to RCTTurboModule before instantiating
Summary: At times, the lookup logic may find a class that's not compliant with RCTTurboModule. If so, it shouldn't be instantiated, and we assume the module doesn't exist.

Reviewed By: JoshuaGross, RSNara

Differential Revision: D13979004

fbshipit-source-id: ac1fa9cc456715cddd101fff13f5a41f9528a74e
2019-02-06 16:10:30 -08:00
Kevin Gozali 3e9f9cfe47 TM iOS: Util to check if an id is TurboModule instance
Summary: Simple macro to do check if TurboModule is enabled and the particular object is RCTTurboModule compliant.

Reviewed By: PeteTheHeat

Differential Revision: D13978368

fbshipit-source-id: 660c7cab7bb074d80d57abead951dad19306ae73
2019-02-06 16:01:31 -08:00
Kevin Gozali 8ae185280d TM iOS: guard against nullptr in module lookup
Summary: Just tightened up the lookup logic to guard against nullptr.

Reviewed By: RSNara

Differential Revision: D13948001

fbshipit-source-id: 55bf46619712e60e195ea12f2b8eb799f80a3bbe
2019-02-04 14:57:18 -08:00
Ramanpreet Nara aa39cb6d9f Remove assertion on TurboModule existence
Summary:
It's okay for TurboModules to not exist. Therefore, `__turboModuleProvider(moduleName)` should return null if `moduleName` doesn't refer to an actual TurboModule. The current implementation assumes that the TurboModule must exist, and therefore, it crashes the app when it doesn't (assertion error).

This change is important because it helps us land D13887962. When we switch `I18nResources` to `TurboModuleRegistry` in D13887962, on Wilde, it won't be found in `NativeModules`. Therefore, we'll try to lookup this module in `__turboModuleProvider`, and this will crash the app. It seems like `I18nResources` is a Java-only module?

Reviewed By: fkgozali

Differential Revision: D13924589

fbshipit-source-id: 7ac7b1873e06852e5aafcaaef5c24cbc548ee444
2019-02-04 11:36:05 -08:00
Ramanpreet Nara 0ceefb40d5 Enable module lookup in TurboModules
Summary:
NativeModules are instantiated by the bridge. If they choose, they can capture the bridge instance that instantiated them. From within the NativeModule, the bridge can then be used to lookup other NativeModules. TurboModules have no way to do such a lookup.

Both NativeModules and TurboModules need to be able to query for one another. Therefore, we have four cases:
1. NativeModule accesses NativeModule.
2. NativeModule accesses TurboModule.
3. TurboModule accesses NativeModule.
4. TurboModule accesses TurboModule.

In summary, this solution extends the bridge to support querying TurboModules. It also introduces a `RCTTurboModuleLookupDelegate` protocol, which, implemented by `RCTTurboModuleManager`, supports querying TurboModules:
```
protocol RCTTurboModuleLookupDelegate <NSObject>
- (id)moduleForName:(NSString *)moduleName;
- (id)moduleForName:(NSString *)moduleName warnOnLookupFailure:(BOOL)warnOnLookupFailure;
- (BOOL)moduleIsInitialized:(NSString *)moduleName
end
```

If TurboModules want to query other TurboModules, then they need to implement this protocol and synthesize `turboModuleLookupDelegate`:

```
protocol RCTTurboModuleWithLookupCapabilities
property (nonatomic, weak) id<RCTTurboModuleLookupDelegate> turboModuleLookupDelegate;
end
```

NativeModules will continue to use `RCTBridge` to access other NativeModules. Nothing needs to change.

When we attach the bridge to `RCTTurboModuleManager`, we also attach `RCTTurboModuleManager` to the bridge as a `RCTTurboModuleLookupDelegate`. This allows the bridge to query TurboModules, which enables our NativeModules to transparently (i.e: without any NativeModule code modification) query TurboModules.

In an ideal world, all modules would be TurboModules. Until then, we're going to require that TurboModules use the bridge to query for NativeModules or TurboModules.

`RCTTurboModuleManager` keeps a map of all TurboModules that we instantiated. We simply search in this map and return the TurboModule.

This setup allows us to switch NativeModules to TurboModules without compromising their ability to use the bridge to search for other NativeModules (and TurboModules). When we write new TurboModules, we can have them use `RCTTurboModuleLookupDelegate` to do access other TurboModules. Eventually, after we migrate all NativeModules to TurboModules, we can migrate all old callsites to use `RCTTurboModuleLookupDelegate`.

Reviewed By: fkgozali

Differential Revision: D13553186

fbshipit-source-id: 4d0488eef081332c8b70782e1337eccf10717dae
2019-01-31 11:35:05 -08:00
Kevin Gozali 8a50bc3ab3 iOS: Make each module implement getTurboModuleWithJsInvoker: instead of having centralized provider
Summary:
For better modularity, each module conforming to RCTTurboModule should provide a getter for the specific TurboModule instance for itself. This is a bit more extra work for devs, but simplify tooling and allow better modularity vs having a central function that provides the correct instance based on name.

Note: Android may or may not follow this new pattern -- TBD.

Reviewed By: RSNara

Differential Revision: D13882073

fbshipit-source-id: 6d5f82af67278c39c43c4f7970995690d4a82a98
2019-01-30 17:32:16 -08:00
Ram N e6f7d69428 Move Android TurboModules to github
Summary:
This commit moves all the turbo module files for Android to Github.

Note that gradle build is not yet enabled.
Sample Turbo Modules will be added in a later commit.

Other missing features

- Support for `CxxModule`
- Remove usage of folly::dynamic for arguments and result conversion
- Support for Promise return types.

Reviewed By: mdvacca

Differential Revision: D13647438

fbshipit-source-id: 5f1188556d6c64bfa2b2fd2146ac72b0fb456891
2019-01-14 15:48:09 -08:00
Kevin Gozali db87c2544c Re-land D13583442: [RN] Moved TurboModule iOS core to github
Summary:
The original commit was backed out due to a build failure, which seems to be due to compiler flag misconfiguration. That has been fixed.

Original commit: f2fccbb327

Reviewed By: mdvacca

Differential Revision: D13593075

fbshipit-source-id: f3a65e8bd9830f6ac2ea829020500b4632ee2149
2019-01-08 09:12:50 -08:00
Kevin Gozali 8c23b1804b Revert D13583442: [RN] Moved TurboModule iOS core to github
Differential Revision:
D13583442

Original commit changeset: bb1cabd973e8

fbshipit-source-id: 0279e68c7001ec91cf7bbd835bf83232943bb708
2019-01-07 00:47:38 -08:00
Kevin Gozali f2fccbb327 Moved TurboModule iOS core to github
Summary:
This is the iOS binding for TurboModule.

To install the TurboModule binding:
* Provide `RCTCxxBridgeDelegate`
* Provide `RCTTurboModuleManagerDelegate`

Somewhere in `RCTCxxBridgeDelegate` impl:

```
RCTTurboModuleManager *manager = [[RCTTurboModuleManager alloc] initWithRuntime:&runtime bridge:bridge delegate:self];
[manager installJSBinding];
```

Doing so will install `global.__turboModuleProxy()` in JS space.

Note:
* The full instructions will be provided once all pieces are moved to OSS.
* Sample modules and binding setup will be provided later.

Reviewed By: RSNara

Differential Revision: D13583442

fbshipit-source-id: bb1cabd973e8a9ec59da6b145826e9ea234a96b3
2019-01-04 15:24:07 -08:00
Kevin Gozali acc2ed2488 Moved TurboModule C++ core to github
Summary:
This is only the core C++ part of TurboModule - moving to github to make integration with existing NativeModules system slightly easier. Other bindings for iOS/Android are not yet ready to move.

Notes:
* TurboModules is not ready to use at the moment.
* Build configuration is not yet provided (cocoapods/.xcodeproj/gradle), just like Fabric.
* No effort was done to make this lib C++17 strictly compliant yet (there will be in the future).

Reviewed By: RSNara

Differential Revision: D13551211

fbshipit-source-id: cd3b458e6746ee9218451962ca65b1ad641a32db
2018-12-26 17:18:21 -08:00