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
Summary:
CatalystInstanceImpl.destroy does a bunch of stuff in each of the relevant threads (Native Module thread, JS thread, UI thread).
This change creates a V1 destroy method (unchanged) and a V2 destroy method. The goal is to resolve (and catch!) race conditions in native modules and JSI modules that could occur during teardown; and mitigate race conditions that occur in RN teardown, like deallocation of C++ objects (scheduler, JS VM, and UIManager for Fabric).
Changelog:
[Internal] Experiment to fix deallocation race conditions
Reviewed By: mdvacca
Differential Revision: D18001677
fbshipit-source-id: 5955da0a7b726491c7d749642475f0fba74cce5a
Summary:
WebSocketModule can be called asynchronously while the ReactContext/CatalystInstance is being torn down. Trying to get a JS module at this time will result in a crash if the ReactContext has already torn down Catalyst. Check explicitly that Catalyst is still alive before trying to emit an event to JS via some JS module.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18020359
fbshipit-source-id: 1b77abd457c7d97bd241389251890bb682b6fde3
Summary:
AppStateModule can be called indirectly when the ReactContext is torn down. Trying to get a JS module at this time will result in a crash if the ReactContext has already torn down Catalyst. Check explicitly that Catalyst is still alive before trying to emit an event to JS via some JS module.
Changelog: [Internal]
Reviewed By: ejanzer, mdvacca
Differential Revision: D17969056
fbshipit-source-id: dd446de57280e588a73f3e228bac82b3d67ecdc0
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:
We want ReactContext to blow up if it tries to use `mCatalystInstance` after `destroy` is called.
Changelog:
[[Internal]]
Reviewed By: mdvacca
Differential Revision: D17944723
fbshipit-source-id: cfe8a8b98473f53dd68bbcd91a71f58bd7a0c503
Summary:
This diff checks in the output from `fbsource//xplat/js:FBReactNativeSpec-flow-type-android-files` into `react-native-github/ReactAndroid/src/main/java/com/facebook/fbreact/specs/`.
To update the codegen output, please run:
```
js1 build oss-native-modules-specs -p android
```
To use the codegen, add the following as a buck dep to your target:
```
fbsource//xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/fbreact/specs:FBReactNativeSpecAndroid
```
Changelog: [Android][Added] Check in TurboModules codegen
Reviewed By: fkgozali
Differential Revision: D17990380
fbshipit-source-id: 94895e375311e9360c308dffadf1558a5ff04212
Summary:
This already has an assert that `destroy` should only be called on the UI thread. Add an annotation.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D17989113
fbshipit-source-id: fd44f321cbcb7f0a18383ca6226cce72e5991eea
Summary:
This diff extends the ReactAppTestActivity to allow customization of the RN ExceptionHandler
Changelog:
This diff extends the ReactAppTestActivity to allow customization of the RN ExceptionHandler
Reviewed By: JoshuaGross
Differential Revision: D17993132
fbshipit-source-id: 7320d35a4d88d56a7356f2863829c88758c08341
Summary:
This PR fixes Android CI.
## Changelog
[Android] [Fixed] - fix Android CI
Pull Request resolved: https://github.com/facebook/react-native/pull/26862
Test Plan: Android CI is green again
Differential Revision: D17925576
Pulled By: mdvacca
fbshipit-source-id: 90bdccda96a11210254a9eaab3c373151057f332
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:
The Android makefiles had hard-coded paths to hermes-engine, sometimes looking in `node_modules` and other times looking in `..`. This commit implements the Node module resolution algorithm (see common.mk), which handles both of these cases and also looks further up the root if necessary, handling the case when the `hermes-engine` npm package is hoisted.
This commit does three things:
- Defines `find-node-module` and uses it in the makefiles to find `hermes-engine`
- Removes the unused `/path/to/hermes-engine/include` paths since this directory does not exist and should be `/path/to/hermes-engine/android/include` (`android`)
- Moves the definition of `REACT_NATIVE` in the makefiles to the top. It was defined after every `$(CLEAR_VARS)` invocation but was not actually cleared anyway. `$(CLEAR_VARS)` resets only `LOCAL_*` variables in this list: https://android.googlesource.com/platform/build/+/7dc45a8/core/clear_vars.mk
## Changelog
[Internal] [Changed] - Android Makefiles look for hermes-engine using Node's module resolution algorithm
Pull Request resolved: https://github.com/facebook/react-native/pull/26820
Test Plan: Run `./gradlew :ReactAndroid:installArchives`, which requires the hermes-engine paths to be correct in order to find the headers.
Differential Revision: D17923671
Pulled By: cpojer
fbshipit-source-id: 9238b8718a94080db1abbba6375a6a1d484c871d
Summary:
Currently the react native framework doesn't handle the accessibility state changes of the focused item that happen not upon double tapping. Screen reader doesn't get notified when the state of the focused item changes in the background.
To fix this problem, post a layout change notification for every state changes on iOS.
On Android, send a click event whenever state "checked", "selected" or "disabled" changes. In the case that such states changes upon user's clicking, the duplicated click event will be skipped by Talkback.
## Changelog:
[General][Fixed] - Announce accessibility state changes happening in the background
Pull Request resolved: https://github.com/facebook/react-native/pull/26624
Test Plan: Add a nested checkbox example which state changes after a delay in the AccessibilityExample.
Differential Revision: D17903205
Pulled By: cpojer
fbshipit-source-id: 9245ee0b79936cf11b408b52d45c59ba3415b9f9
Summary:
The Gradle build file looks up jsc-android and hermes-engine using hard-coded paths. Rather than assuming the location of these packages, which are distributed and installed as npm packages, this commit makes the Gradle file use Node's standard module resolution algorithm. It looks up the file hierarchy until it finds a matching npm package or reaches the root directory.
## Changelog:
[Android] [Changed] - ReactAndroid's Gradle file uses Node's module resolution algorithm to find JSC & Hermes
Pull Request resolved: https://github.com/facebook/react-native/pull/26773
Test Plan: Ensure that CI tests pass, and that `./gradlew :ReactAndroid:installArchives` works. Printed out the paths that the Gradle script found for jsc-android and hermes-engine (both were `<my stuff>/react-native/node_modules/jsc-android|hermes-engine`).
Differential Revision: D17903179
Pulled By: cpojer
fbshipit-source-id: 9ac3ba509974f39f87b511d5bc3398451c12393f
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: UseVanillaJNI flag was missing for measure and baseline functions
Reviewed By: amir-shalem
Differential Revision: D17868201
fbshipit-source-id: 95d6843d643e90157a51550d6efbf059f0ca2c39
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:
Add UI thread assertions and annotations to ReactRootView. Shouldn't have any immediate effect since these methods already call other methods that assert they're on the UI thread. Doing this to hoist assumptions higher up.
Doing some very simple refactoring to the way cleanup happens to ensure aggressive cleanup in more instances.
Reviewed By: shergin
Differential Revision: D17860291
fbshipit-source-id: c87e0336594fa2327271b648eb8340e250235250
Summary: We can use getCurrentEnv() instead of passing environment variable around
Reviewed By: amir-shalem
Differential Revision: D17842042
fbshipit-source-id: 185b174ae7c08e746bc76c0600c2e326b15c4993
Summary: We can use getCurrentEnv() , no need to pass env around methods
Reviewed By: amir-shalem
Differential Revision: D17841281
fbshipit-source-id: a4a58292e70ac05b0f0b9eb962a82a8501ad0226
Summary: Move JNI_ONLoad to a separate file so that both fbjni native methods and vanillla jni native methods can be initialized correctly
Reviewed By: amir-shalem
Differential Revision: D17840166
fbshipit-source-id: 045df0df7a95bc331cbbefb3a118a349f3029465
Summary: set missing useVanillaJNI, it was causing part of the unit-tests to run in fbjni instead of vanillajni.
Reviewed By: SidharthGuglani
Differential Revision: D17852635
fbshipit-source-id: 5bc187d90fbdc430015be55a015a7d1e0ba0ebc6
Summary: This PR caused a regression with shadows, see https://github.com/facebook/react-native/issues/26544.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D17809320
fbshipit-source-id: 0c83cd211425622ada0fd8c492f43df0536a4b8a
Summary: Use TestParametrization to test both fbjni and vanilla jni versions
Reviewed By: amir-shalem
Differential Revision: D17788718
fbshipit-source-id: 0f3317b7403cadca7b7ccd9140f1933d746bf433