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:
Looking at the crash reports from T46487253:
1. This crash happens only with TurboModule-compatible NativeModules.
2. Users who experience this crash are in the TurboModules test group.
Therefore, the crash happens while trying to load TurboModules.
The stack trace of the crash includes [this lookup via the NativeModule system](https://fburl.com/diffusion/vxj9goz5). When TurboModules are enabled, we can only start executing this line if one of two things are true:
1. The TurboModuleRegistry is null in CatalystInstanceImpl.
2. The TurboModuleRegistry isn't null but the NativeModule returned by the TurboModuleRegistry is null.
We can protect against 1 by asserting that when `ReactFeatureFlags.useTurboModules` is `true`, `mTurboModuleRegistry` is not null. Once this check lands, unless there's a race with setting `ReactFeatureFlags.useTurboModules`, we should be able to rule out 1.
Changelog:
[Added][Android] - Assert TurboModuleRegistry isn't null before using it in CatalystInstanceImpl
Reviewed By: PeteTheHeat
Differential Revision: D18211935
fbshipit-source-id: de88c033425c474ef80b73386b7182b1d3bb382f
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:
The `StatusBarManager` NativeModule does not have a uniform API on iOS and Android. In particular, the `setStyle` and the `setHidden` methods have an additional parameter on iOS:
```
/**
* - statusBarStyles can be:
* - 'default'
* - 'dark-content'
* - 'light-content'
*/
+setStyle: (statusBarStyle?: ?string, animated: boolean) => void;
/**
* - withAnimation can be: 'none' | 'fade' | 'slide'
*/
+setHidden: (hidden: boolean, withAnimation: string) => void;
```
If we keep the NativeModule spec the same between the two platforms, we'd have to keep the second parameter optional for both methods. This works for `setHidden`, because the second parameter is a string, and optional strings are allowed. However, for `setStyle`, the second parameter is a number, and we don't support optional numbers/booleans on Android in the NativeModule system. If we keep the optional number, then the following check triggers in our RedBox tests on iOS, which makes them fail: https://fburl.com/diffusion/b7adezd9.
So, since the two specs are sufficiently different, I figured that the easiest path forward is to split them apart.
Changelog:
[iOS][Changed] - Separated NativeStatusBarManager into NativeStatusBarManager{IOS,Android}
Reviewed By: PeteTheHeat
Differential Revision: D18214161
fbshipit-source-id: 6fd8b8c5f576244b5b90ee47faa7f50508c5e1d3
Summary:
Using compiler flag -fvisibility=hidden and explicitly setting visibility to default to public methods
#Changelog:
[Internal] [Yoga] Use compiler flag -fvisibility=hidden for reducing yoga binary size
Reviewed By: astreet
Differential Revision: D18029030
fbshipit-source-id: 545e73f9c25f3108fc9d9bb7f08c157dbc8da005
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:
Will not show native redboxes when LogBox handles them
Changelog: [Internal]
Reviewed By: motiz88
Differential Revision: D18236608
fbshipit-source-id: 1c60c69419b1a823594caf650d67693d4ad2076b
Summary:
Text paddings in Fabric are managed by LayoutMetrics, the current implementation of Fabric is incorrectly using padding data from Text Props to set the padding in the view.
This diff refactors the update of Text in Fabric in order to avoid using padding prop data
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D18211638
fbshipit-source-id: de05e7daa6185d854ce1b6580a1e44ae55d3176e
Summary:
We no longer need to gate by OS version since we want to allow in-app theming. This diff ensures that we are passing in the updated system context to retrieve the correct app theme.
Changelog:
[Android] Enable AppearanceModule for all OS versions
Reviewed By: mdvacca
Differential Revision: D18224915
fbshipit-source-id: 42d5db8497d8bead32c49e3e2a25d4ba779e2b33
Summary:
Mostly for easing open-source migration and not making a backwards-incompatible change (yet), we'll set this to false by default. Every app can opt-in to this if wanted but it's not necessary. This change is part of experiments surrounding more-aggressive teardown for Fabric and Bridgeless mode.
Changelog: [Internal] - This has the effect of (by default) disabling the previous diff which caused ReactContext teardown to always set mCatalystInstance to null. Now that is opt-in behavior and off by default, so it's not longer a breaking change.
Reviewed By: mdvacca
Differential Revision: D18207302
fbshipit-source-id: 7acfc894415e966f652c7049849eef79c440a135
Summary:
This PR https://github.com/facebook/react-native/pull/22723 cached selections, so if you had a cached selection indicies, but updated the text to be an empty string, then this would crash.
As reported in https://github.com/facebook/react-native/issues/25265 and other issues of `setSpan(4 ... 4) ends beyond length`
## Changelog
[Android] [fixed] - Crash in TextInput
Pull Request resolved: https://github.com/facebook/react-native/pull/26680
Test Plan:
```
input.setNativeProps({ text: "xxx", selection: {"begin": 0, "end": 3}});
input.setNativeProps({ text: ""});
```
Differential Revision: D18189703
Pulled By: cpojer
fbshipit-source-id: 67d9615a863fd22598be8d6d4553dec5ac8837ed
Summary:
See T55861104. In rare cases if `removeReactInstanceEventListener` is called right after (like, a small number of CPU instructions later, on a different thread) we allocate the `listeners` array with a certain size, then we could have one or more `null` listeners in the array, which is what we've been seeing in prod, at very low volumes, for several years. Without solving the root of the race condition we can just add a null check here.
Maybe it's also possible that if `addReactInstanceEventListener` is called on another thread in a racey way, that the size will be incremented on the array before we can access the additional member. That seems crazy, but maybe.
While this has been firing for multiple years it seems like a more recent change caused a regression. This diff doesn't address that and only resolves the crash.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D18192801
fbshipit-source-id: c1000cfcdf6f251b03061d1386eabb9f0617a7d3
Summary:
Because the `mCatalystInstance` of the ReactContext can be null during teardown, there are technicaly cases where `UIManagerHelper.getUIManager` can return null. In those cases we check for a CatalystInstance and raise a SoftException, and return null. We must then guard in every case where we call `getUIManager` to prevent NullPointerExceptions.
See T56103679.
Currently crashes are coming from `PropsAnimatedNode.restoreDefaultValues` calling `UIManagerModule.synchronouslyUpdateViewOnUIThread` on teardown/at the end of an animation as RN is being torn down.
This can happen in both Paper and Fabric.
In dev this will still crash because the SoftException will trigger a crash. It will be a noop with logged warnings in production builds.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D18165576
fbshipit-source-id: 7059e04ca339208dd64a0a08a375b565cb8cda02
Summary:
In previous diffs I migrated many (all?) NativeModules in FB and open-source to check for `hasActiveCatalystInstance` before calling `getJSModule`. We log SoftExceptions in those cases to find more potential race condition and lifecycle bugs without crashing.
In this diff, I migrate all the non-NativeModule callsites that I could find.
Previous diffs: see D18032458, D18035359, D18032788, D18092136, D18092137, D18112989, D18134400
Changelog: [Internal]
Reviewed By: mdvacca, mmmulani
Differential Revision: D18134694
fbshipit-source-id: 4729abfb84280b634463b1cd9b4dd808f310b6e7
Summary:
Simplify the API of `getReactApplicationContextIfActiveOrWarn`. We don't need to pass so much information into this method to collect good SoftExceptions.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18134400
fbshipit-source-id: 0a250ab0252a44121f3339a31506a0a6c4c7cd35
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:
This diff extends the rendering on Text on Android to support textBreakStrategy prop.
Changelog: Add support for Text.textBreakStrategy prop into RN Android for Fabric
Reviewed By: JoshuaGross
Differential Revision: D18101403
fbshipit-source-id: c7f0b1cdc0de05172f0978d4dd3493620dcd941a
Summary:
In D18032458 we introduce getReactApplicationContextIfActiveOrWarn. In this diff, modules that access a JS or Native module through ReactApplicationContext need to check if the CatalystInstance is still alive before continuing.
Modules that don't derive from `ReactContextBaseJavaModule` manually check for the catalyst impl and log their own SoftExceptions.
In this diff we also introduce SoftExceptions that by contract never cause crashes, even in debug mode.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18112989
fbshipit-source-id: 868f01f388aa2db3518db9f873f2afc2a62eed45
Summary:
When testing out the NetworkOverlay, I noticed that we were creating a lot of WebSocket connections for localhost:8097. Rick found that this is because we're trying to connect to React devtools every 2 seconds: https://github.com/facebook/react/blob/master/packages/react-devtools-core/src/backend.js#L67 and it appears we create a new WebSocket every time.
Dan suggested that we use opening the dev menu as a trigger for attempting to connect to React devtools. This diff uses RCTNativeAppEventEmitter to emit an event from native when the dev menu/dialog is shown, and listening to that event in JS to attempt to connect to devtools.
I'm also making the change of passing in a websocket instead of just passing in the host + port; this way it will only attempt to connect once on each call to `connectToDevTools` (otherwise, we would attempt to reconnect every 2 seconds as soon as the dev menu is opened, and then the next time the menu is opened we'd so start that *again*, and so on - I could have it keep track of whether it's already connecting and avoid doing it again, but this is easier and should be sufficient, I think).
We should probably also update the suggested troubleshooting tips on the devtools page to reflect this change, so that people don't get confused.
Changelog: [General] [Fixed] Fix issue where we attempt to connect to React devtools every 2 seconds
Reviewed By: mmmulani
Differential Revision: D17919808
fbshipit-source-id: 4658d995c274574d22f2f54ea06d7f29ef2f54dc
Summary:
setStyleInputs batching API was added to reduce the number of jni calls and although it improved performance in yoga world but was not impactful in litho and is not used anywhere.
Removing this saves around 500 bytes per architecture
#Changelog:
[Internal][Yoga] Removed unused code setStyleInputs batching API form Yoga
Reviewed By: amir-shalem
Differential Revision: D18036536
fbshipit-source-id: 7436b55dcd464dd9f9cc46406d4fd78d12babe55
Summary:
`responseType` should be a string, not an Object (which gets converted to a NativeMap by TM).
Changelog: [General] [Fixed] Fix the flow type for NativeNetworkingModule
Reviewed By: fkgozali
Differential Revision: D18019418
fbshipit-source-id: 316470ca82241223eafb5b05a54fc2bbf3074821
Summary:
Text font size should not be negative, this diff throws an exception if TextView uses negative of zero font size
Changelog: [[internal]]
Reviewed By: shergin
Differential Revision: D18068071
fbshipit-source-id: 4074dca2019b6223eef68a407570258adbceaa43
Summary:
Text didn't support horizontal textAlign in Fabric for Android, this diff fixes that
Changelog: Add support for TextAlign prop in Fabric
Reviewed By: makovkastar
Differential Revision: D18068072
fbshipit-source-id: 3f8b1ba46989b55197fe9aa60ba2fb055b003d67
Summary:
There is a mixed usage of `folly::make_unique` and `std::make_unique`. Soon, `folly::make_unique` may be removed (see [this PR](https://github.com/facebook/folly/pull/1150)). Since `react-native` only supports C++14-compilers and later, switch to always using `std::make_unique`.
## Changelog
[Internal] [Removed] - Replace folly::make_unique with std::make_unique
Pull Request resolved: https://github.com/facebook/react-native/pull/26730
Test Plan:
Running the existing test suite. No change in behavior is expected.
Joshua Gross: buck install -r fb4a, make sure MP Home and forced teardown works okay on android
Reviewed By: shergin
Differential Revision: D18062400
Pulled By: JoshuaGross
fbshipit-source-id: 978ca794c7e972db872a8dcc57c31bdec7451481
Summary:
Renders frames in RedBox in a greyed-out style when their `collapse` field is set to `true`. This avoids outright hiding information in the stack trace while still drawing attention to frames that are likely to be more meaningful.
Changelog: [General] [Changed] - Render collapsed JavaScript frames in RedBox
Reviewed By: rickhanlonii
Differential Revision: D18039438
fbshipit-source-id: 527588f11c0bff495842be7036cd1293bab65eb9
Summary:
This diff adds support for negative letterspacing values in Fabric android
Changelog: add support fot negative letterspacing values in Fabric android
Reviewed By: JoshuaGross
Differential Revision: D18054648
fbshipit-source-id: de1b906e6b8c0b021ffdc2e4bdad243075230667
Summary:
In D18032458 we introduce `getReactApplicationContextIfActiveOrWarn`. In this diff, modules that access a JS or Native module through ReactApplicationContext need to check if the CatalystInstance is still alive before continuing.
Changelog: [Internal]
Reviewed By: furdei
Differential Revision: D18032788
fbshipit-source-id: 5152783afd0b93b8ce0970fe4a509ea71396a54a
Summary:
In three previous diffs (D18020359 D17998627 D17969056), I implemented this logic in three different modules. There are potentially hundreds of modules where we should be implementing this check, so I'm moving the important logic into ReactContextBaseJavaModule.
Additionally, `WebSocketModule` was retaining its own copy of ReactApplicationContext instead of using the built-in `getReactApplicationContext`, so I removed that ivar from `WebSocketModule`.
Changelog:
[Internal]
Reviewed By: mdvacca
Differential Revision: D18032458
fbshipit-source-id: 9114120d3b80334df8d2e0813e36d21c667fc1bd
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:
In `tearDownReactContext`, `reactContext.destroy()` sets `mCatalystInstance` to null. We cannot call `reactContext.getCatalystInstance()` after that without hitting an assertion. Change ordering so that doesn't happen in reload or teardown.
Changelog: [Internal]
Reviewed By: makovkastar
Differential Revision: D18041279
fbshipit-source-id: 22658dc506b76cf58aee1008841abacfe9410c9d
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
David Vacca
Oleksandr Melnykov
Ramanpreet Nara
Sidharth Guglani
Rick Hanlon
Elisa Lou
Joshua Gross
Maurus Cuelenaere
Marco Munizaga
Yoga Setiawan
Dulmandakh
Emily Janzer
Joe Loser
Moti Zilberman
