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:
Collecting Telemetry is a crucial part of building a performant UI framework; we do that but we need to improve it to make the data more reliable, actionable and trustful.
Now we collect time points as the number of milliseconds from the start of the CLOCK_MONOTONIC epoch. That's fine but it also has problems:
Sometimes a millisecond is an eternity. We have only 16 (or fewer) of them on each frame. What if some operation takes 1ms (according to telemetry) but we have to run it a dozen times? Does it mean that it's 12 ms in total? So, we lack precision.
This is not type-safe. Do you know how many milliseconds in a microsecond? I don't. We multiply that on magical constants hoping that we copied that from some other place right.
The current implementation is not cross-platform. We have ifdefs for iOS and Android and Unix and Windows (which is now implemented).
So, this diff replaces that with using `std::chrono` which is part of the standard library that designed to fix all those concerns. We also define our type-aliases on top of that to express our concrete constrains:
We use `std::chrono::steady_clock` as the base clock which is according to the standard using `clock_gettime(CLOCK_MONOTONIC, ... )` if available. So, it's fast and compatible (the same under the hood) with Android infra.
We use nanoseconds when we store time durations (TelemetryDuration type).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D19184569
fbshipit-source-id: 7a44688f4bb3bfc6e3009874f0075c531c8569a1
Summary:
This is an incomplete effort to migrate from libfb to libfbjni. This is needed to restore the compatibility with Flipper and other FB Android projects that make use of FBJNI. Effectively, the outcome is that `fbjni` no longer has a checked-in copy here, but instead relies on the public artifacts published at github.com/facebookincubator/fbjni that can be deduplicated at build-time.
**A non-exhaustive list of tasks:**
* [X] Gradle builds the SDK and RNTester for Android.
* [X] Buck build for rntester works in OSS.
* [ ] Move from `java-only` release to full `fbjni` release. This requires finding a solution for stripping out `.so` files that the old `Android.mk` insists on including in the final artifacts and will clash with the full distribution.
* [ ] Import this and fix potential internal build issues.
* [ ] Verify that the changes made to the Hermes integration don't have any unintended consequences.
## Changelog
[Android] [Changed] - Migrated from libfb to libfbjni for JNI calls
Pull Request resolved: https://github.com/facebook/react-native/pull/27729
Test Plan:
- CI is already passing again for Gradle and Buck in OSS.
- After applying the following patch, RNTester builds and works with the latest Flipper SDK:
```
diff --git a/RNTester/android/app/build.gradle b/RNTester/android/app/build.gradle
index b8a6437d7..eac942104 100644
--- a/RNTester/android/app/build.gradle
+++ b/RNTester/android/app/build.gradle
@@ -170,10 +170,19 @@ dependencies {
debugImplementation files(hermesPath + "hermes-debug.aar")
releaseImplementation files(hermesPath + "hermes-release.aar")
- debugImplementation("com.facebook.flipper:flipper:0.23.4") {
+ debugImplementation("com.facebook.flipper🐬+") {
exclude group:'com.facebook.yoga'
- exclude group:'com.facebook.flipper', module: 'fbjni'
- exclude group:'com.facebook.litho', module: 'litho-annotations'
+ exclude group:'com.facebook.fbjni'
+ }
+
+ debugImplementation("com.facebook.flipper:flipper-network-plugin:+") {
+ exclude group:'com.facebook.yoga'
+ exclude group:'com.facebook.fbjni'
+ }
+
+ debugImplementation("com.facebook.flipper:flipper-fresco-plugin:+") {
+ exclude group:'com.facebook.yoga'
+ exclude group:'com.facebook.fbjni'
}
if (useIntlJsc) {
```
Reviewed By: mdvacca
Differential Revision: D19345270
Pulled By: passy
fbshipit-source-id: 33811e7f97f44f2ec5999e1c35339909dc4fd3b1
Summary:
LocalData isn't used by any components. We've moved towards State.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D19250737
fbshipit-source-id: 10bf0b62ffad01ad10b07d029e84df4f312780a1
Summary:
Ran `arc f` against some CPP files to be modified with the next diff
to reduce the churn on it.
## Changelog
[Android] [Changed] - Formatted cpp/h code with clang-format
Reviewed By: javache
Differential Revision: D19371785
fbshipit-source-id: b7f7b92c4cb9ec7f8da728bb577db29cf11fbb39
Summary:
This diff promotes the UIManagerModule.getEventDispatcher() to the interface UIManager and it implements this method in FabricUIManager class.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18862862
fbshipit-source-id: 424f0e601ed1807dbd5d33048daf7dc3bb200dcd
Summary:
Motivation: TextInput.js frequently sends commands to views as they're disappearing (`blur`, for instance).
We should fix that in the future, if possible. For now, just log the issue and continue. It shouldn't cause any user-facing issues since 1) it appears that TextInput knows the underlying view is gone; 2) the View has already disappeared so the user can't interact with it, so commands can go safely into the void.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18821448
fbshipit-source-id: 980dbbce8cdb2cc0bb4bf60b3bccc90869208f01
Summary:
We hope that will help us to understand more.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18599164
fbshipit-source-id: 431f83de707fc7113e04abd3dd5b59ee5c9cc675
Summary:
This diff exposes the surfaceID as part of ReactRootView.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18474014
fbshipit-source-id: 98f651b211d3cc4df88c6b1eb257bc12129eff57
Summary:
Document which methods can be called on UI thread or ANY thread.
In the future we should see if we can use only `ThreadConfined` or the AndroidX annotations instead of using both / choosing between them at each site.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18532542
fbshipit-source-id: 3b5406ea5035615a0ebf83484bf8ec0747a6b6f7
Summary:
Fixes T54997838 by preventing any view mutations during `onMeasure` calls.
There might still be places where this is possible, but this is where I'm seeing all the crashes currently.
See comments in ReactRootView for why views were mutated during onMeasure.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18518591
fbshipit-source-id: 1406af8a6b0bfcc86f4cc5b451b3967f312dfd85
Summary:
Easy replace of int -> Tag type in Binding
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18429188
fbshipit-source-id: 08a19dff60bd90585f59a284be0e46ee7f381bcc
Summary:
This diff fixes the algorithm that collapses mutation instructions in Fabric Android
The problem was that we were always relying on the fact that the oldShadowNode is present as part of the mutation instruction (which is not correct for mutation instructions of type CREATE)
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D18411360
fbshipit-source-id: 36f5e75f792db87003fcaef2ddcd9452415a0ad6
Summary:
This diff updates the configuration of collapsing of mounting instructions to be disabled by default (in case the user didn't get any value from the server)
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18411934
fbshipit-source-id: 21f9ca525c28134800e929407c5a1e29a84de68e
Summary:
This diff changes the implementation a little bit:
* We don't use RuntimeExecutor there anymore (because it's already been used in EventBeat). The actual purpose of EventBeatManager was distilled to "provide a tick synchronized with the main run loop";
* Now we use dedicated EventBeatManagerObserver interface to decouple this functionality from EventBeat-intrinsic functionality;
* A bunch of small clean-ups.
* Now we ensure threading and capturing ownership in a single function in AsyncEventBeat class. Before this change, the EventBeatManager called `beat` function of the EventBeat directly bypassing AsyncEventBeat class, and effectively bypassing the check that underlying infra still exists (aka EventBeat::OwnerBox feature). That alone might fix some C++ crashes that we see.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18307655
fbshipit-source-id: 3b582cb71085ed99ee94f8e6d575196c2082557b
Summary:
* We no more pollute the global namespace with JSI stuff;
* We don't use JSI here, so why to include;
* We don't use `Instance`, so why to forward-declare this;
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D18307654
fbshipit-source-id: b6c3ad55d7bd605074db9fa1d4bc58a73e0b2d74
Summary:
The C++ counterpart of EventBeatManager uses RuntimeExecutor for that, so it's redundant to do that in EventBeatManager.java.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18307657
fbshipit-source-id: 65823b25d691a56540247d317e3c0e86685150c8
Summary:
Now the function supports iOS.
We will use it soon.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D18285828
fbshipit-source-id: b9c16433e9c75ab4c071e4bd55074757372f6c0a
Summary:
This diff fixes the Collapsing of Delete-Create mounting instructions algorithm. By mistake I switch the conditions before landing the original diff.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D18306616
fbshipit-source-id: 50cd8ca67adcaf172ce51896df684fed270b2d51
Summary:
This diff introduces the flag IS_DEVELOPMENT_ENVIRONMENT that will be used in Fabric to control the logging of props, localData and state ONLY during development.
Using DEBUG mode to control the logging of this kind of data is not enough.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D18290351
fbshipit-source-id: cf0824bd15b9f1c509bbb284b85761166099bc42
Summary:
This diff implements an optimization / fix in the mounting layer of Fabric Android to ignore the "deletion" and "creation" of views for the same tag in the same commit.
This operation is adding ~100 ns for every commit (I measured this using a release APK running in a real device). I created a QE to enable / disable this optimization and to measure the performance impact of this change in production
Changelog: Implement optimization in mounting layer of Fabric
Reviewed By: JoshuaGross
Differential Revision: D18279240
fbshipit-source-id: d6fdeb2a9676bcfaf47886893eed5024bf86204b
Summary:
This diff adds a new parameter in Binding class to configure the collapsing of Delete-Create Mounting instructions. This is necessary to fix T55696973.
I'm configuring this in order to measure the cost of this fix in produiction environment.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D18279239
fbshipit-source-id: b7743f6364b66d19c9ae7309919926debf574213
Summary:
Easy diff to extend logging in FabricUIManager class
Changelog: Add extra logging in Fabric Android
Reviewed By: shergin
Differential Revision: D18277487
fbshipit-source-id: 387bdb4b237bdbdc0d65263c1f125ad5c9e26b18
Summary:
Quick diff to avoid the logging of Props, State and localData in Fabric Android.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D18277486
fbshipit-source-id: 462335e7dadaab2bd39a8ede6318f52f95dfb53a
Summary:
easy diff to avoid the constant copy of a vector when calling the method Binding.createRemoveAndDeleteMultiMountItem. Since we are not modifing the vector inside the method createRemoveAndDeleteMultiMountItem it's not necessary to copy it.
Changelog: Improve performance in mounting of Fabric views
Reviewed By: JoshuaGross
Differential Revision: D18250376
fbshipit-source-id: c984214a8148bab521cec51d42ba54a4b73e3e67
Summary:
This diff annotates FabricUIManager class with NonNull annotations, this will help analysis of nullability plus improving integration with Kotlin clients
Changelog: Add NonNull annotation to FabricUIManager API
Reviewed By: JoshuaGross
Differential Revision: D18010917
fbshipit-source-id: 760ba04b78693cb184172c0fe613c7f808a49031
Summary:
This diff refactors the execution of the logging of Fabric React markers to be executed after the MountItems are executed on the UI Thred
Changelog: Improve logging of Fabric react markers
Reviewed By: JoshuaGross
Differential Revision: D18010920
fbshipit-source-id: e36306102d190119a89c16e660b855acab1528fe
Summary:
This diff refactors the stopping of DispatchUIFrameCallback on FabricUIManager to make it thread safe
Changelog: Refactor the cancellation of dispatching of Mounting operations for Fabric
Reviewed By: JoshuaGross
Differential Revision: D18010922
fbshipit-source-id: 305bc65576698cb785a2a2308cbd03db4a9a97e4
Summary:
This diff annotates core classes of Fabric with NonNull and Nullable annotations, this will help analysis of nullability plus improving integration with Kotlin clients
Changelog: Add NonNull annotation to Fabric core classes
Reviewed By: shergin
Differential Revision: D18010918
fbshipit-source-id: 40fe68470b97cdf740f52dfeb9130465aab5e6df
Summary:
This diff annotates Fabric MountingManager and Events classes with NonNull annotations, this will help analysis of nullability plus improving integration with Kotlin clients
Changelog: Add NonNull annotation to Fabric Event classes
Reviewed By: shergin
Differential Revision: D18010923
fbshipit-source-id: fb9d5683bbd51fa25dda9b2023f9c411c3ff541d
Summary:
This diff annotates MountItems classes with NonNull annotations, this will help analysis of nullability plus improving integration with Kotlin clients
Changelog: Add NonNull annotation to Fabric MountItems
Reviewed By: JoshuaGross
Differential Revision: D18010921
fbshipit-source-id: 4c2bded87f7af1ddb941b2a49e390e51984890c0
Summary:
sDidInit can be accessed from different threads, this diff refactors the definition of this variable to be volatile and also to be assigned at the end of the staticInit() method.
Changelog:
Ensure proper initialization of FabricSoLoder
Reviewed By: ejanzer
Differential Revision: D18010919
fbshipit-source-id: 3ec7b19fdc15056b90fc01281b8c3888e93a7dd3
Summary:
This diff adds an experiment to disable the preallocation of views on the mounting layer of Fabric
Changelog:
Add a ReactNativeConfig to configure the preallocation of views in the mounting layer of Fabric
Reviewed By: shergin
Differential Revision: D17949681
fbshipit-source-id: 0af63df22aff9e94289bc8a8217c79222f1fd61c
Summary:
While unlikely, in exceptional cases it's possible that `onCatalystInstanceDestroy` is called multiple times, especially since there's no restrictions as to which thread this is called on (currently it's only called on the JS thread from CatalystInstanceImpl) and no synchronization. Make sure it tears down cleanly if called multiple times, and report soft exceptions if that's detected.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18001678
fbshipit-source-id: 4a7c37b5b3c2207ba1dd9c7d85690391f799518d
Summary:
This diff changes updateState method to support null stateWrappers, before this method would crash with a NullPointerException, now it allows the null object to reach the view manager.
Changelog: Add support for update of nullable localState in Fabric
Reviewed By: JoshuaGross
Differential Revision: D17939651
fbshipit-source-id: c62555905e39f9e0db75b9e1d1b93f33d0560266
Summary: Tear down FabricUIManager more aggressively. The rest of the ivars here we can't set to null (yet) because they're marked as final.
Reviewed By: mdvacca
Differential Revision: D17878342
fbshipit-source-id: a1c8ace2603750ac39c3d97e1aca6c486ba5fb79
Summary: These methods are (or should be) only called on the UI thread. Make those assumptions explicit.
Reviewed By: mdvacca
Differential Revision: D17865205
fbshipit-source-id: 9b3acf8f3215a07b1a667ced55e50e99a488de79
Summary: Although `mBinding` is unregistered which means the connection to the JNI-bridged Cxx object can be destructed, we still hold onto the `mBinding` Java object after unregistering. That doesn't seem desirable, I think we should just clear it out here for consistency.
Reviewed By: mdvacca
Differential Revision: D17865206
fbshipit-source-id: 1ad8643c48ba0b2d52620a7b8ebe8a52928648ef
Summary:
We use `ViewManager.onMeasure` to perform measurements of Android views and pass the measured size back to Yoga. For Android in order to the report correct dimensions of a View, this View must be created using a Context that has a theme associated with it. Before, `onMeasure` only had ReactApplicationContext passed as the first parameter and ReactSwitch, for example, could not be measured correctly (because it uses the size of the thumb drawable, which is extracted from the current theme). This diff adds surfaceId as the first parameter of `FabricUIManager.measure`, so that we can retrieve ThemedReactContext and pass it to `ViewManager.onMeasure`.
The size of the Switch component is still incorrect, but at least the size reported back to Yoga is the same as in Paper. So there is more investigation necessary why this happens in Fabric. I will investigate and publish another diff with the fix.
Reviewed By: JoshuaGross, shergin
Differential Revision: D17625959
fbshipit-source-id: 48197a61240fb13042bef3e9f5d681acacc702fb
Summary: This diff fixes the issue with view paddings in Fabric introduced in D17081799. In Paper setting padding on a view was only supported for Text and TextInput components (see https://fburl.com/codesearch/6r6lu5vd). We want to keep Fabric backwards compatible, so we delegate setting the padding to the view manager instead of setting it directly on the view. In this way we can have a no-op implementation in the base view manager and implement this method only in view managers that support setting padding (ReactTextInputManager and ReactTextViewManager at the moment).
Reviewed By: JoshuaGross
Differential Revision: D17665011
fbshipit-source-id: 38bc56278e002bd34881cfcb9ed79df579f79e28
Summary: This diff makes the Video component compatible with Fabric on Android.
Reviewed By: JoshuaGross
Differential Revision: D17450815
fbshipit-source-id: 5e47d8cdca96f9fbe72902bac7dd157f7b5fdb1a
Emily Janzer
Valentin Shergin
Pascal Hartig
Samuel Susla
Oleksandr Melnykov
Pascal Hartig
David Vacca
Joshua Gross
Andres Suarez