Summary: Easy diff to add extra debug information in the CreateMountItem class. This will be useful to debug bugs in Fabric
Reviewed By: JoshuaGross
Differential Revision: D16381362
fbshipit-source-id: 22073df228908b6c88e6423c4917fc6d64c73f98
Summary: Supporting View Manager Commands on the new UIManager in Fabric. This is needed for things like scrollTo on ScrollView.
Reviewed By: JoshuaGross
Differential Revision: D16175575
fbshipit-source-id: a74effdf7e47b56a150a4e3fb6c4d787659e0250
Summary: Right now we register ReactFabric as a callable module with the bridge so that we can call `ReactFabric.unmountComponentAtNode` in `ReactInstanceManager.detachViewFromInstance`. In bridgeless mode we don't have callable modules, so I'm just setting a global variable that can be called from C++ instead. Using this in a new `unmount` method in FabricUIManager.
Reviewed By: shergin, mdvacca
Differential Revision: D16273720
fbshipit-source-id: 95edb16da6566113a58babda3ebdf0fc4e39f8b0
Summary:
The method MountingManager.removeRootView is not used anymore D16275118, I'm deleting from MountingManager
motivation: cleanup as I'm fixing other bugs in Fabric
Reviewed By: ejanzer
Differential Revision: D16350582
fbshipit-source-id: 488cf2aa2feda78e6660a854af5da2718f9905de
Summary:
FabricUIManager.removeRootView() isn't currently used, removing it from the UIManager interface.
It looks like this is called from JS in paper renderers, but not Fabric, so we should be good to delete it.
Reviewed By: shergin, mdvacca
Differential Revision: D16275118
fbshipit-source-id: b8f3ae1dc7574ce17d8cc9e7fee72ef5dcc9b323
Summary:
Step 1 in removing the dependency on ReactContext from GuardedRunnable and other related classes. These are extensively by native modules and view managers, so in order to remove the bridge dependency from those modules we'll need to first decouple these classes from ReactContext. It turns out they only need ReactContext for its handleException method, which delegates out to product code. For backwards compatibility I'm exposing another NativeModuleExceptionManager in ReactContext that simply wraps its handleException method (since this interface already does everything we need).
I figured I'd keep around an extra constructor that still uses ReactContext for now instead of trying to migrate everything over at once.
Reviewed By: makovkastar
Differential Revision: D16270995
fbshipit-source-id: c9a8714bea7ac2a98e78234a0bae49140c00980d
Summary: This diff makes the Binding.commitMutex_ recursive, for more context see: D15995971
Reviewed By: shergin
Differential Revision: D16187220
fbshipit-source-id: 559d06e1e4a83d7beee13f41b4549db2b91983e5
Summary:
This diff migrates the usages Nullable and NonNull annotations to AndroidX instead of javax.
The purpose of this change is to bring consistency in the annotations used by the core of RN
Reviewed By: makovkastar
Differential Revision: D16054504
fbshipit-source-id: 21d888854da088d2a14615a90d4dc058e5286b91
Summary: Now, the signature of `updateState` method practically copies the signature of `updateLocalData`. We need that to support all features that `updateLocalData` does support now (to migrate from it).
Reviewed By: mdvacca
Differential Revision: D15962377
fbshipit-source-id: 61e0af6c191e0c6a358c5859613e9c512f91d29a
Summary: Originally, moving the mount instruction generation under the `if` was a perf optimization but now, since we converge `LocalData` and `State`, this is no longer possible (because we need to treat State as LocalData in some cases).
Reviewed By: mdvacca
Differential Revision: D15962378
fbshipit-source-id: 37f9fadb72ac53450c2d499452610d9835f2964d
Summary: This diff prevents the execution of MountItems without mounting instructions
Reviewed By: JoshuaGross
Differential Revision: D16063571
fbshipit-source-id: 97ff93c6880c7d8857f61d4998df4327f41fafdd
Summary: The method FabricUIManager.scheduleMountItems receives only one MountItem, is makes more sense to be called FabricUIManager.scheduleMountItem
Reviewed By: JoshuaGross, makovkastar
Differential Revision: D16062749
fbshipit-source-id: a27063be33b644af83ede6a9198edbfb1c3296e1
Summary: This diff exposes LayoutDirection as part of UpdateLayoutMountItem
Reviewed By: JoshuaGross
Differential Revision: D16060521
fbshipit-source-id: 163bf2a0bdca62dcecb03a8aaa2f4bf595b18c8f
Summary: This diff ensures only one thread access the Binding.schedulerDidFinishTransaction method
Reviewed By: shergin
Differential Revision: D15995971
fbshipit-source-id: 31f03803e8829480dd30b9b9148fd09b218ebeab
Summary:
This diff formats the Java class files inside xplat/js/react-native-github. Since google-java-format was enabled in D16071401 we want to codemode the existing code so that users don't have to deal with formatter lint noise at diff-time.
```arc f --paths-cmd 'hg files -I "**/*.java"'```
drop-conflicts
Reviewed By: cpojer
Differential Revision: D16071725
fbshipit-source-id: fc6e3852e45742c109f0c5ac4065d64201c74204
Summary:
Right now JS triggers a view manager command with the following code:
```
UIManager.dispatchViewManagerCommand(
ReactNative.findNodeHandle(this),
UIManager.getViewManagerConfig('RCTView').Commands.hotspotUpdate,
[destX || 0, destY || 0],
);
```
As we want to get rid of calls to UIManager, we need to stop looking for the integer defined in native from JavaScript. We will be changing methods like this to be:
```
UIManager.dispatchViewManagerCommand(
ReactNative.findNodeHandle(this),
'hotspotUpdate',
[destX || 0, destY || 0],
);
```
We need to support ints and Strings to be backwards compatible, but ints will be deprecated.
Reviewed By: shergin
Differential Revision: D15955444
fbshipit-source-id: d1c488975ae03404f8f851a7035b58a90ed34163
Summary: Removing the UiThread annotation from MountingManager.addRootView() because it doesn't seem like it's needed.
Reviewed By: mdvacca
Differential Revision: D15945819
fbshipit-source-id: 0cb9bfb2fe2889001f1c061a07518cf9734f0c16
Summary: This diff adds extra logging in FabricUIManager, this will be useful to debug production issues
Reviewed By: JoshuaGross
Differential Revision: D15907520
fbshipit-source-id: 94e16444af3c023b6c4837c4797404d3debe8e95
Summary: Previously, we didn't provide any LayoutContext to the very first commit (aka startSurface); apparently it causes problems.
Reviewed By: mdvacca
Differential Revision: D15891472
fbshipit-source-id: b2a785bdad5764eb97b41f5bf5679807107bd8a0
Summary: I added this function in D15810990, not sure why lint didn't complain about this...
Reviewed By: zackargyle
Differential Revision: D15880129
fbshipit-source-id: 4d207ba26be46186dc3904c82d82a4a83d4d4eb7
Summary:
It is possible that race conditions exist in access of scheduler and UIManager in Binding.cpp, such that deallocated or null schedulers/UIManagers could be passed as arguments into various functions, causing strange and exotic crashes down the line once garbage memory (or null pointers) are dereferenced.
I think it's possible that navigating away from a Fabric screen while there's high memory pressure could cause things to get deallocated more aggressively and cause scheduler and/or UIManager to disappear while mounting instructions are being created, for instance.
Reviewed By: mdvacca
Differential Revision: D15863193
fbshipit-source-id: a85cb71d88ea54826b40f44e788931dfc422f045
Summary: This diff forces the method: scheduler.constraintSurfaceLayout to run on the JS thread.
Reviewed By: JoshuaGross
Differential Revision: D15845768
fbshipit-source-id: de2aa69f301770aaf6cb7c3f5670548a3b6110df
Summary: Scheduler.startSurface accepts LayoutConstraints and LayoutContext, but for some reason we don't use these on Android. This diff adds new methods to Binding and FabricUIManager to start a surface with the provided measurespecs. I created new methods to avoid affecting the functionality of any surfaces already using Fabric, but if we want this behavior everywhere then I can just add it to the existing `addRootView` and `startSurface`.
Reviewed By: JoshuaGross, shergin
Differential Revision: D15810990
fbshipit-source-id: 6cd9a58b125461f91253458905405298cfb723ce
Summary: Just adding a few systrace sections to add perf visibility in a few more areas.
Reviewed By: ejanzer
Differential Revision: D15823394
fbshipit-source-id: 6ac1773cbd8e01a747ad015726e16c0bf0e8596b
Summary: Easy diff to create a ReactFeatureFlag to enabled logging in Fabric
Reviewed By: JoshuaGross
Differential Revision: D15803582
fbshipit-source-id: d735f24850bddf43c27b97d006100cbb8f0cc6e3
Summary:
... and slighly new behaviour for one of them.
The method does nothing if given `key` already exists in the container.
This diff finishes the transition of ContextContainer from an internal bag of things with unclear yet ownership into a legit dedicated dependency injection container for the product code.
The original names of methods imply that the container can have only one object of a given type which is no longer true. The new API is much more generic and idiomatic to C++, it mimics `std:map` API which is intuitive to anyone who familiar with C++ containers.
Besides the naming, `insert` method changed the semantic a bit; now it does nothing in case of inserting an object with a key that already exists. That might seem counterintuitive for "normal" people, but C++ has some wired reasons for that and, hopefully, it's expected behavior in the C++ community.
Fun fact: We need this to fix hot-reload.
Reviewed By: sahrens
Differential Revision: D15681736
fbshipit-source-id: 194f342528446a911eaf072ba3a94a5d8af3cb52
Summary:
We currently have two different codepaths for actually rendering a surface with Fabric on iOS and Android: on iOS we use Fabric's `UIManagerBinding.startSurface` to call `AppRegistry.runApplication`, but on Android we don't; instead we use the same codepath as paper, calling `ReactRootView.runApplication`.
This diff does a few different things:
1. Unify iOS and Android by removing the `#ifndef` for Android so that we call `startSurface` for both
2. Pass through the JS module name on Android so that this actually works (it currently passes in an empty string)
3. Remove the call to `ReactRootView.runApplication` for Fabric so that we don't end up doing this twice
4. Copy over some logic that we need from `ReactRootView.runApplication` (make sure that root layout specs get updated, and that content appeared gets logged)
Reviewed By: mdvacca
Differential Revision: D15501666
fbshipit-source-id: 5c96c8cf036261cb99729b1dbdff0f7c09a32d76
Summary: trivial diff to remove warnings because of the lack of Nullable annotations in MountingManager.ViewState
Reviewed By: shergin
Differential Revision: D15476040
fbshipit-source-id: 2b9a4efa1be1d5aa29d4e32cf32c8ff502f7c60c
Summary: This is an optimization to avoid transfering updateState instructions twice during the frist render of a view (same a props)
Reviewed By: shergin
Differential Revision: D15476041
fbshipit-source-id: 8a62035dbbb63c93f86a2f8d217986a325cb1805
Summary: This diff fixes the rendering of Bottom Sheet in Fabric Android. In D15343702 we added state as part of the "preallocateView" method but we forgot to call viewManager.updateState(), this prevents the state to be updated during the first render.
Reviewed By: shergin
Differential Revision: D15476042
fbshipit-source-id: cd6fc9bdd178589d2e04f85723425b5e5c3e5a04
Summary: This class is not necessary anymore, this diff deletes it from the repo
Reviewed By: JoshuaGross
Differential Revision: D15457346
fbshipit-source-id: c7293d93b50271efe3b3d2121c128ba6e13c7627
Summary:
Right now calling FabricUIManager.addRootView() doesn't actually start running the application on Android. This diff:
1. Removes the #ifndef so that we actually call UIManagerBinding.startSurface() on Android
2. Passes through the JS module name from addRootView so we can render the surface (falls back to an empty string if not provided, which is the current behavior)
3. Adds an option for starting the surface using `RN$SurfaceRegistry` instead of `AppRegistry`, if that global property has been defined in JS. This is used for Venice (bridgeless RN)
Reviewed By: shergin
Differential Revision: D15366200
fbshipit-source-id: 4a506a589108905d4852b9723aac6fb0fad2d86e
Summary:
I just learned about Nuclide's auto-formatting (cmd-shift-c) and started using it in another diff, but I didn't want to pollute the diff with a bunch of formatting changes, so here we are.
I don't know if anyone else uses Nuclide's auto-formatting, or something else - happy to ditch this if that's not how we roll.
Reviewed By: shergin
Differential Revision: D15389601
fbshipit-source-id: e3b20acd073adf3cc7bab1f62d86c5b5dab8c4fc
Summary:
For some components, we will have state as soon as the ShadowNode is created that may be meaningful. In those cases, ViewManagers should be able to use State to create or preallocate views.
FB: This will be used in following diffs for Litho support.
Reviewed By: mdvacca
Differential Revision: D15343702
fbshipit-source-id: 8fd672251cb88dea662b5cae5a9efc96877d28a9
Summary:
Easy diff to refactor the sComponentNames map out of the FabricUIManager class.
This is a necessary clean-up to perform a slightly major refactor of the Fabric classes
Reviewed By: JoshuaGross
Differential Revision: D15421769
fbshipit-source-id: 3be73a6e20b338c8cea23ef0c88db417df7e3aa9
Summary:
Quick diff to refactor RootTag for surfaceId in Binding.cpp class
This is the first diff to start moving away from rootTag naming in Fabric
Reviewed By: JoshuaGross
Differential Revision: D15421770
fbshipit-source-id: 7bca7782f96be3d7148ee93f5d5a3a54e0d768dd
Summary:
The `measure` API receives LocalData and Props, it should also receive State.
This will also be used in future diffs.
Reviewed By: mdvacca
Differential Revision: D15325182
fbshipit-source-id: 6cb46dd603ce7d46673def16f0ddb517e2cf0c4f
Summary: `YogaStylableProps.yogaStyle` is designed to be consumed by Yoga only. Making it `protected` allows us to avoid confusion and misuse of this props.
Reviewed By: JoshuaGross
Differential Revision: D15296474
fbshipit-source-id: cf9e416afee99fb426d72765557b34d303a63dbe
Summary:
This diff forces the eager initialization of some additional classes into FabricJSIModuleProvider.loadClasses().
This is a "hack" that will be removed in the near future
Reviewed By: JoshuaGross
Differential Revision: D15208977
fbshipit-source-id: 2e2c7856839b6c6888452800ef6da7f269e46735
Summary: When passing StateWrapper objects across the JNI, we were not ensuring that the Java objects would own the C++ state. This was initially done because I assumed that in Java, State would either be used immediately or discarded, so this wouldn't be unsafe. As it turns out, it makes sense in some cases to store the StateWrapper in Java and use it later, potentially even in other threads, so we need to make sure we maintain ownership of the C++ object from the Java object.
Reviewed By: shergin
Differential Revision: D15206194
fbshipit-source-id: a437d921ba00b194cf08bad80666bd99baf11d52
Summary: This diff implements encapsulating all time metrics in a single class for better extensibility and readability.
Reviewed By: JoshuaGross
Differential Revision: D15179835
fbshipit-source-id: 62bdf94435a0d37a87ad9bad613cc8e38043a235
David Vacca
Eli White
Emily Janzer
Valentin Shergin
Oleksandr Melnykov
Joshua Gross