Summary:
Adding a `.clang-tidy` to a bunch of dirs under `react-native-github/ReactAndroid` and `react-native-github/ReactCommon`.
I don't want to add a single `.clang-tidy` at the root because we'll need more fine-grained control over what checks are enabled in different parts of the codebase; for example, fabric will and TM will probably have more checks enabled than older parts of the codebase that we're not actively modernizing, and the Hermes team probably wants its own config to be consistent with the rest of their codebase.
Starting off each `.clang-tidy` by only enabling clang-diagnostic; this is just to test that it's working. In the future, we'll work with the community to gradually enable more checks.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19705749
fbshipit-source-id: 979cea053b645ac4a9790340033bfcfb49ca0f97
Summary:
## Motivation
The concept behind JSCallInvoker doesn't necessarily have to apply only to the JS thread. On Android, we need to re-use this abstraction to allow execution of async method calls on the NativeModules thread.
Reviewed By: PeteTheHeat
Differential Revision: D17377313
fbshipit-source-id: 3d9075cbfce0b908d800a366947cfd16a3013d1c
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
Summary: In D15252375 I made JSCallInvoker an abstract class and extended it in BridgeJSCallInvoker, but I forgot to add the `override` keyword for overridden methods.
Reviewed By: fkgozali
Differential Revision: D15328292
fbshipit-source-id: 3e75faf1b4a968b80643b8a97071ab2e122fd643
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
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
Summary:
`CatalystInstanceImpl.cpp` now depends on `JSCallInvoker` and `JavaJSCallInvokerHolder`. Therefore, we need to correctly adjust the OSS builds to include these dependencies into the `libreactnativejni.so` file.
I made `ReactCommon/turbomodule/jscallinvoker` a static library `libjscallinvoker.a`. I then made `ReactAndroid/src/main/jni/react/jni`'s `libreactnativejni.so` depend on that static library. Also, because the Android NDK build system doesn't support header namespaces, I had to use the filesystem to simulate them. This is why all the `.cpp` and `.h` files for `JSCallInvoker` were moved to a `jsireact` folder.
Reviewed By: mdvacca
Differential Revision: D15194821
fbshipit-source-id: 0cee682e41db53d0619f56ad017d5882f6d554aa
Summary:
`TurboModuleManagerDelegate` is an abstract base class with the following API:
```
public TurboModule getModule(String name, ReactApplicationContext reactApplicationContext);
public CxxModuleWrapper getLegacyCxxModule(String name, ReactApplicationContext reactApplicationContext);
```
```
std::shared_ptr<TurboModule> getTurboModule(std::string name, jni::global_ref<JTurboModule> turboModule, std::shared_ptr<JSCallInvoker> jsInvoker) override;
std::shared_ptr<TurboModule> getTurboModule(std::string name, std::shared_ptr<JSCallInvoker> jsInvoker) override;
```
On the C++ side, when asked to provide a TurboModule with name `name`:
1. First, is this a CxxModule? If so:
1. Create the CxxModule and return
2. Otherwise:
1. Somehow get a Java instance of the TurboModule
2. If this Java object represents a CxxModule, like `FbReactLibSodiumModule`:
1. Grab the C++ part of this object, and wrap it in a `TurboCxxModule.cpp` and return.
3. Otherwise:
1. Wrap the Java object in a C++ HostObject and return.
This pseudocode demonstrates how we'd use `TurboModuleManagerDelegate` to implement `__turboModuleProxy`.
```
__turboModuleProxy(name, jsCallInvoker):
let cxxModule = TurboModuleManagerDelegate::getTurboModule(name, jsCallInvoker)
if (!cxxModule) {
return cxxModule;
}
// JNI Call that forwards to TurboModuleManagerDelegate.getLegacyCxxModule
let javaCxxModule : CxxModuleWrapper = TurboModuleManager.getLegacyCxxModule(name)
if (!javaCxxModule) {
return std::shared_ptr<react::TurboCxxModule>(javaCxxModule.getModule())
}
// JNI Call that forwards to TurboModuleManagerDelegate.getModule
let javaModule : TurboModule = TurboModuleManager.getModule(name)
if (!javaCxxModule) {
return TurboModuleManagerDelegate::getTurboModule(name, javaModule, jsCallInvoker)
}
return null
```
Reviewed By: mdvacca
Differential Revision: D15111335
fbshipit-source-id: c7b0aeda0e4565e3a2729e7f9604775782b6f893
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
Emily Janzer
Ramanpreet Nara
Kevin Gozali
James Treanor
Emily Janzer