Summary:
Trivial.
Apparently, `DEBUG` is non-standard feature and using `assert` with `DEBUG` is practically asking for bugs. So, if your `assert` relies on some variable which is only defined when `DEBUG` is set, it's easy to get invalid code because NDEBUG and DEBUG can be unsync.
So, we have to use clunky double negative `#ifndef NDEBUG` everywhere where we used DEBUG.
Reviewed By: JoshuaGross
Differential Revision: D15031328
fbshipit-source-id: 036f573e68925741ca46384261885766c87db1e3
Summary:
@public
RCTScrollView sometimes asserts with another contentView set. While this doesn't crash, it'd be good to know what's causing the assert. This will add a recursive description of the contentView.
Changelog: [iOS] [Changed] RCTScrollView: added debugging to help fix assert
Reviewed By: PeteTheHeat
Differential Revision: D15049869
fbshipit-source-id: 5431de7764881922327c6c0a3bdd392668396b58
Summary:
@public
Introduces `YGNodeConstRef` as `const YGNode*`, i.e. a pointer to a constant `YGNode`.
We also use it for all style getters, which will avoid casts to `const YGNode*` in diffs up the stack.
We should use this pointer type for all functions that do not modify the underlying node.
Reviewed By: SidharthGuglani
Differential Revision: D14999095
fbshipit-source-id: 61cc53bb35e787a12ae12e70438d84c0a4983752
Summary:
@public
In order to encapsulate property access on `YGStyle`, as a first measure we wrap all fields with accessors.
This will e.g. enable dynamic property storage and instrumentation in the future.
All accessors have a `const` version that allows direct access via `const&`. For mutation, bit fields are wrapped with a custom reference object.
This style allows for the least amount of changes in client code. Property access simply needs appended parens, eg `style.direction` becomes `style.direction`.
Reviewed By: shergin
Differential Revision: D14999096
fbshipit-source-id: fbf29f7ddab520513d4618f5e70094c4f6330b30
Summary:
@public
Remove unnecessary `const` and `extern` specifiers from `Yoga.h`.
- Function declarations are `extern` by default
- The removed `const` specifiers for pass-by-valye parameters are only meaningful for the *definition* of functions, not for the declaration.
In this specific case, I found `const YGNodeRef` particularly confusing, as it is a `typedef` for a pointer type. `const` does not refer to the pointed-to object, but to the parameter itself, i.e. `const YGNodeRef` is `YGNode * const`, and not `const YGNode *`.
Reviewed By: SidharthGuglani
Differential Revision: D14999097
fbshipit-source-id: 8350870cb67f4a34722f796c4f4a2fc7dde41b99
Summary:
Simplifies the code anyone on iOS using RN has _to see_. In this case, React Native knows that everyone probably wants all these imports (unless they're using dev mode) and so we can auto-import the pod specs for a user from inside the lib.
Basically auto-link for the React side.
[iOS] [Added] - Adds a ruby function which imports the Pods for RN, so that users only have to include this function and it can change per RN version.
Pull Request resolved: https://github.com/facebook/react-native/pull/24555
Differential Revision: D15044780
Pulled By: cpojer
fbshipit-source-id: c3702a52104706def51da6f1d11ab966d57d1edb
Summary:
React Native Environment Info:
System:
OS: Linux 4.15 Ubuntu 18.04.1 LTS (Bionic Beaver)
CPU: (4) x64 Intel(R) Core(TM) i5-7300U CPU @ 2.60GHz
Memory: 1.12 GB / 15.55 GB
Shell: 4.4.19 - /bin/bash
Binaries:
Node: 8.10.0 - /usr/bin/node
Yarn: 1.13.0 - /usr/local/bin/yarn
npm: 3.5.2 - /usr/bin/npm
SDKs:
Android SDK:
API Levels: 16, 19, 22, 23, 24, 25, 26, 27, 28
Build Tools: 23.0.1, 23.0.3, 25.0.0, 25.0.2, 25.0.3, 26.0.1, 26.0.3, 27.0.3, 28.0.2, 28.0.3
System Images: android-16 | Google APIs Intel x86 Atom, android-19 | Google APIs Intel x86 Atom, android-24 | Google Play Intel x86 Atom, android-27 | Google APIs Intel x86 Atom, android-28 | Intel x86 Atom_64, android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom
npmPackages:
react: 16.8.6 => 16.8.6
react-native: git+https://github.com/facebook/react-native.git#v0.59.5 => 0.59.5
npmGlobalPackages:
react-native-cli: 2.0.1
react-native-git-upgrade: 0.2.7
The workaround implemented in https://github.com/facebook/react-native/pull/21117 tries to fix
https://issuetracker.google.com/issues/112385925 scroll direction (according to the latest comments, the scroll direction problem has been reverted in security patches so not sure if the workaround is still valid).
But... proposed solution in fling method is using signum which leads to zero computedVelocityY in case of zero mOnScrollDispatchHelper.getYFlingVelocity() on old devices(Samsung s4 mini) even when real velocityY is non zero
```
final int correctedVelocityY = (int)(Math.abs(velocityY) * Math.signum(mOnScrollDispatchHelper.getYFlingVelocity()));
```
Proposed solution is to take signum from original velocityY in case of zero
```
float signum = Math.signum(mOnScrollDispatchHelper.getYFlingVelocity());
if (signum == 0) {
signum = Math.signum(velocityY);
}
final int correctedVelocityY = (int)(Math.abs(velocityY) * signum);
```
The symptoms are the same as described in issue https://github.com/facebook/react-native/issues/22925, but proposed workaround doesn't work.
[Android][fixed] - Fix smooth scrolling on old devices (SDK >=16)
Pull Request resolved: https://github.com/facebook/react-native/pull/24545
Differential Revision: D15044834
Pulled By: cpojer
fbshipit-source-id: 3f523eb1a438df774e22387aecded433b9031ab9
Summary:
By default, the text color is `#000000` on iOS and different on Android, e.g. `#808080`, depending on the manufactorer.
This PR changes it so that newly created projects all have the text color `#000000` by default on both iOS and Android.
The argument for this is to make the app by default be more consistent between platforms.
Expo also does this: https://github.com/expo/expo/blob/master/android/expoview/src/main/res/values/styles.xml#L31
---
For context and for your consideration, I have started a discussion here with the topic of whether React Native should try to use OS defaults or be consistent between platforms:
https://github.com/react-native-community/discussions-and-proposals/issues/121
[Android] [Changed] - New projects have a `#000000` by default.
Pull Request resolved: https://github.com/facebook/react-native/pull/24540
Differential Revision: D15044898
Pulled By: cpojer
fbshipit-source-id: 3197266504e1061ac7027bec3100e39e39a4406a
Summary:
By default, an alert is `cancelable` on Android but not on iOS.
This PR changes the behavior so that the Alert is not dismissable on Android by default.
The motivation is that many developers develop on iOS and test on Android, and do forget to consider the case that the alert is dismissable.
Consistent behavior by default makes it easier to develop cross-platform apps in general.
---
For context and for your consideration, I have started a discussion here with the topic of whether React Native should try to use OS defaults or be consistent between platforms:
https://github.com/react-native-community/discussions-and-proposals/issues/121
---
If this PR gets merged, the docs should be updated as well:
https://github.com/facebook/react-native-website/blob/master/docs/alert.md#android
[Android] [Changed] - By default, alerts are not dismissable
Pull Request resolved: https://github.com/facebook/react-native/pull/24541
Differential Revision: D15044798
Pulled By: cpojer
fbshipit-source-id: 762b2ace69eb7ec79cd6ebec916e473348b9cafe
Summary:
Fixes a regression in https://github.com/facebook/react-native/commit/1f8b46a2fc191e2899735fbdcdbb535a91b63724. The internal subscription vendor uses a sparse array to track listeners, which makes listener removal fast. When querying listeners, the sparse entries need to be removed. `Array#filter` is a built-in way to do this -> linked to the JS spec, which explains this.
[General] [Fixed] - Fixed sparse array handling in `EventEmitter#listeners()`
Pull Request resolved: https://github.com/facebook/react-native/pull/24546
Differential Revision: D15044790
Pulled By: cpojer
fbshipit-source-id: 0f1301618739357b4a0f5378b9584efe74f0f09a
Summary:
If an app [builds from RN source](https://facebook.github.io/react-native/docs/building-from-source), there was an error for jsc-android not found.
It is a side effect of my previous [JSC as node dependency change](https://github.com/facebook/react-native/commit/8e375850de73d1b998a83279e6c4ba238867a453)
For building from RN source case, the jsc-android is located at `/path/to/app/node_modules/jsc-android`.
Original gradle task will try to find it at `/path/to/app/node_modules/react-native/ReactAndroid/../node_modules/jsc-android`, as ReactAndroid project path was being override inside node_modules.
The change fixes the building from source case.
N/A
This change does not need to publish into changelog, as it is a master branch building fix.
Pull Request resolved: https://github.com/facebook/react-native/pull/24547
Differential Revision: D15044703
Pulled By: cpojer
fbshipit-source-id: a7d824b1a14064d46c4a2ec9ea28255179174c83
Summary:
Bumps to Xcode 10.2.0 and uses the iOS 12.2 simulator when running tests.
The `testBundleURL` test is temporarily disabled to allow the iOS unit tests to run successfully in the internal Facebook CI system.
Changelog:
[iOS] [Changed] - iOS tests: Bump Xcode to 10.2.0, iOS to 12.2.
Reviewed By: TheSavior
Differential Revision: D14962710
fbshipit-source-id: 769cfb90aacce33903ab6e8dbcc5b5727deacf41
Summary: `jClassName_` is unnecessary because you can use `JNIEnv::GetObjectClass` to get the TurboModule's Java class.
Reviewed By: fkgozali
Differential Revision: D14937480
fbshipit-source-id: 2c1c9be53217331152270dbac3d13f372a2ed818
Summary:
libreactnativejni does not need to link against libjsc directly:
that library is only needed by libjscexecutor.
Reviewed By: mhorowitz
Differential Revision: D15017942
fbshipit-source-id: fb545c9127221dab28434321fe9c116fe72f7507
Summary:
UAs must adjust border radius values to fit a content box:
>>> Corner curves must not overlap: When the sum of any two adjacent border radii exceeds the size of the border box, UAs must proportionally reduce the used values of all border radii until none of them overlap.
This diff implements that.
Reviewed By: mdvacca
Differential Revision: D15028325
fbshipit-source-id: 368232ffa2fa0409d13759bbbe7fe10f8474c400
Summary: Additional check verifies that requested type matches the type of a stored value.
Reviewed By: mdvacca
Differential Revision: D14944336
fbshipit-source-id: 6d3a1654d9b9a64ced83f553236093f02f2c97c7
Summary:
ShadowTree commits happen concurrently with limited synchronization that only ensures the correctness of the commit from ShadowTree perspective.
At the same time artifacts of the commit () needs to be delivered (also concurrently) to the proper thread and executed in order (not-concurrently). To achieve this we need some synchronization mechanism on the receiving (mounting) side. This class implements this process.
Practically, this diff fixes a problem with glitching UI during the very first render of Fabric screen.
Reviewed By: JoshuaGross
Differential Revision: D15021794
fbshipit-source-id: 62982425300c515e92b91e1e660b45455a5446e9
Summary:
`MountingTransaction` encapsulates all artifacts of `ShadowTree` commit, particularly list of mutations and meta-data.
We will rely on this heavily in the coming diffs.
Reviewed By: JoshuaGross
Differential Revision: D15021795
fbshipit-source-id: 811da7afd7b929a34a81aa66566193d46bbc34f8
Summary: We have to figure out a different way to request for a fallback component in ComponentDescriptorRegistry and in general, in public APIs. But now, to stop crashing here the fix.
Reviewed By: JoshuaGross
Differential Revision: D15021796
fbshipit-source-id: a60c66838e76ace990f2eb764c86c29d24db2141
Summary: `getContextContainer` should be marked as const so that const instances can call it.
Reviewed By: shergin
Differential Revision: D14969981
fbshipit-source-id: 8812f24ecf0642a38496580689943fbd43cddad1
Summary:
Removes the top-level return used in `run-android-ci-instrumentation-tests.js`.
This prevents the file from being parsed as a module. The top-level return here doesn't serve any purpose because it doesn't block the execution of any code after it, as there is no more code to execute.
Reviewed By: cpojer
Differential Revision: D15000395
fbshipit-source-id: 7acf010bb25dec5048813d01517875bcb6eb5a15
Summary: It seems like ReactContext isn't actually needed in measure functions. Changing the signature of ViewManager.measure() to take a Context instead.
Reviewed By: lunaleaps
Differential Revision: D14940330
fbshipit-source-id: b29987fd1d7f9c191a5f26138151082ca61cb351
Summary:
Registries, providers, providers of registries, registres of providers. All that can be really confusing, but that represents best the constraints and desires that we have:
* We need to be able to register components on-the-fly (so we need a mechanism that will propagate changes);
* We don't want to register ComponentDescriptors separately from ComponentView classes;
* C++ not always gives us abstractions that we want (e.g. pointers to constructors).
After the change, we can remove the whole Buck target that has a bunch of handwritten boilerplate code.
There is a still room for polish and removing some concepts, types or classes but this diff is already huge.
Reviewed By: JoshuaGross
Differential Revision: D14983906
fbshipit-source-id: ce536ebea0c905059c7a4d644dc25233e2809761
Summary:
ComponentDescriptorProvider represents unified way to create a particular descriptor.
Now all ComponentViews (which support RCTComponentViewProtocol) expose a `ComponentDescriptorProvider` which will allow creating and registering ComponentDescriptor instances for all visual components automatically as a part of ComponentView registration process.
Don't panic, everything is still being as explicit as it always was, no magic involved; we just will have only one registration step instead of two parallel.
That also opens a way to register components on the fly.
Reviewed By: JoshuaGross
Differential Revision: D14963488
fbshipit-source-id: 9e9d9166fabaf7b30b35b8647faa6e3a19cd2435
Summary: The bridge was not properly isolating isInspectable onto the JS thread.
Reviewed By: fkgozali
Differential Revision: D14991970
fbshipit-source-id: 92a06c90bade8f92bfa81fa3b7dfb23b17db6117
Summary: This installs the sample module to the RNTester.xcodeproj without using any TurboModule infra. This is possible because SampleTurboModule is backward compatible with the existing NativeModules system. This also fixes CI test failure: https://circleci.com/gh/facebook/react-native/84752
Reviewed By: RSNara
Differential Revision: D14987572
fbshipit-source-id: f5f2c4330c7f6558c7d4beeb43198869090dee02
Summary:
Makes sure `onViewableItemsChanged` fires ASAP when `waitForInterations` is false.
This also works around another deeper bug where updates scheduled with `InteractionManager` aren't firing at all in some cases, and thus instead of just firing late, `onViewableItemsChanged` isn't firing until scroll which is not what we want with `waitForInterations: false`. That bug will require more digging.
Differential Revision: D14984333
fbshipit-source-id: 718b39670307c6bc16268759bdb513682745265d
Summary:
Yargs package is used only in test scripts and script for bumping version.
Based on this https://github.com/facebook/react-native/search?q=yargs&unscoped_q=yargs Yargs is not really required in production build and should be moved as development dependency.
[General] [Changed] - Moved Yargs to devDepencies
Pull Request resolved: https://github.com/facebook/react-native/pull/24496
Differential Revision: D14982834
Pulled By: cpojer
fbshipit-source-id: 81b6be413b2037e1b852cba91f4a2c04267ae930
Summary:
This diff fixes the reconnect logic with the metro websockets which is causing the app to not re-connect when metro crashes. To demonstrate the issue, consider the following video:
{F156029086}
On the left we have metro, on the right is the xcode console with some logging to show the reconnecting phase. When we kill the metro server you can see the app tries to reconnect once and that's it - when metro is started back up, you can see the notification that there are no apps running and can also see that cmd+opt+r doesn't work anymore
I updated the logic to optimistically start the connection and if it's still unavailable to retry again after the timeout
[iOS][Fixed] - Metro websocket reconnect logic
Reviewed By: shergin
Differential Revision: D14961433
fbshipit-source-id: 0569aa169dc9f538a7e4a8d04e99de39f2e9b3f9
Summary:
Motivation:
* We don't use them much, and we already have `at`-methods, which are better.
* We don't want to expose `ComponentDescriptor`s as shared pointers (because it's not clear, not so performant, and because we don't want to store them as shared pointer in the future);
* In idiomatic C++ `[]` operator has mutating semantic, that's not what we want to communicate via the interface of the class.
Reviewed By: sahrens
Differential Revision: D14963487
fbshipit-source-id: dbfddee2ba90d70c3bb8dcf1959d553571c47bab
Summary:
React Native has a `NativeModule` to manipulate programmatically the dev menu options (live reload, hot reload, remote debugging, etc), called [`DevSettings`](https://github.com/facebook/react-native/blob/master/React/Modules/RCTDevSettings.mm#L120). However this module is only available for iOS.
This PR brings the same `DevSettings` for Android, making it a cross-platform NativeModule.
Motivation: Right now if your app needs to programmatically reload RN, one option is to install [`react-native-restart`](https://www.npmjs.com/package/react-native-restart). It's a tiny dependency, but it's annoying to have to install it, while the code to do so is inside RN codebase. According to NPM, react-native-restart has ~12k weekly downloads, shows it's a recurring feature for many apps (my case).
Thus making `NativeModules.DevSettings` is a small increment in the codebase, just exposing the dev menu methods, to improve the Development Experience
[Android] [Added] - Add DevSetting native module (making it cross-platform)
With expection of `setIsShakeToShowDevMenuEnabled`, the following methods will be available for both platforms:
* reload
* setHotLoadingEnabled
* setIsDebuggingRemotely
* setIsShakeToShowDevMenuEnabled
* setLiveReloadEnabled
* setProfilingEnabled
* toggleElementInspector
Pull Request resolved: https://github.com/facebook/react-native/pull/24441
Differential Revision: D14932751
Pulled By: cpojer
fbshipit-source-id: 465e6a89c3beb5fd1ea22e80ea02e9438f596a09
Summary:
This updates `detox` from `12.1.1` to `12.2.0`.
It includes (amongst other things) the following fixes, as described on the [12.1.4 release page](https://github.com/wix/Detox/releases/tag/12.1.4):
- hotfix: redundant logs in the folder
- fix: stabilize simulator log recording
It also updates the detox test script and config in `packages.json` (which was previously missed when detox was updated to 12.1.x). It will remove the following deprecation warnings:
```
detox[6416] WARN: [test.js] Deprecation warning: "file" and "specs" support will be dropped in the next Detox version.
detox[6416] WARN: [test.js] Please edit your package.json according to the migration guide: https://wix.to/I0DOAK0
```
Instead of extra preprocessing the test folder by `detox`, it will now be passed straight to `jest`.
More information in the [migration guide](https://github.com/wix/Detox/blob/master/docs/Guide.Migration.md).
[General] [Changed] - Update detox to 12.2.0
Pull Request resolved: https://github.com/facebook/react-native/pull/24425
Differential Revision: D14973771
Pulled By: cpojer
fbshipit-source-id: 367ed1f75d2cc3b02349282e7a6201684ba130a7
Summary:
In order to meet our accessibility requirements we need to have full support for keyboard navigation. The Touchable components works with press/tap with a finger, but doesn't respond to 'enter' when using a keyboard. Navigation works fine. This PR adds an onClick listener to touchable views that have the onPress prop defined.
[Android] [Added] - Add View.OnClickListener to Touchable components when onPress is defined
Pull Request resolved: https://github.com/facebook/react-native/pull/24359
Differential Revision: D14971230
Pulled By: cpojer
fbshipit-source-id: ca5559ca1308ee6c338532a00dcea4d00fa57f42
Summary:
This change adds a document outlining the main stakeholders in React Native and their responsibilities: Partners, core contributors and community contributors
It serves as an overview for companies and individuals who are interested in contributing, to get a better understand of how the project is set up, who supports it, and what they do.
This is the first version and I am hoping to integrate it into more places in the future as well as I'd like to keep it up-to-date as things change. Hector is working on a detailed document outlining what it means to be a core contributor and we'll make a list of existing core contributors soon as well.
Pull Request resolved: https://github.com/facebook/react-native/pull/24493
Differential Revision: D14973910
Pulled By: cpojer
fbshipit-source-id: 3a722582be6bb3b953f10073682c5841cdae2c27
Summary:
To ensure greater type safety, we want to generate some cpp tests for the fromRawValue conversions
This diff adds support to generate Tests.cpp along with the `buck test` targets itegrated into the rn_codegen rule automatically. The tests just `assert(true, true)` as a starting point
Reviewed By: fkgozali
Differential Revision: D14739493
fbshipit-source-id: fc9dea64ea31e6af7d997aebc54cfd459d48bf4f
Summary: This is removing packages and libraries from the repo. Any modified buck files simply change the redirect targets to something more appropriate (no logic actually changed)
Differential Revision: D14950721
fbshipit-source-id: 6c14f827b76ca1dbaf83dcb983930f362c6a27d4
Summary:
Second attempt at adding border properties to `RCTImageView`.
Previous attempt can be found at D14875673 which was reverted.
1. `UIImageView` is no longer laid out in in `layoutSubviews`.
2. `updateWithImage` was not being called, I instead of calling `self.image = image` I was calling `_imageView.image = image` directly which skips calling `updateWithImage`. This meant that image's rendering mode was not changed to `template`.
Reviewed By: shergin
Differential Revision: D14934103
fbshipit-source-id: b74c692f9f8ad520ef1f9c70ec4b4aa68b868cd4
Summary:
If you call into a Java method (from C++ using JNI) that raises an exception, the JNI call won't actually raise a C++ error. Instead, the `JNIEnv` will record the pending Java exception and the C++ will continue executing as normal. This is bad because the next time you call into JNI, the app will actually crash, unless you explicitly cleared the exception using `JNIEnv::ExceptionClear()` before the JNI call.
With respect to TurboModules, we need to make sure that RedBoxes show up whenever a native methods raise an exception. We also don't want the app to crash when a JNI method call fails because of a raised exception. Therefore, in this diff, I raise a C++ exception if `JNIEnv::ExceptionCheck()` is true.
Reviewed By: mdvacca
Differential Revision: D14738540
fbshipit-source-id: 4c3063aa93ae7aef025bd2dab6b45059bb8fb409
Summary:
If the return type of a TurboModule method is `Promise`, the infra should create a `com.facebook.react.bridge.Promise` object and pass it as the final argument of the TurboModule Java method call. The Java TurboModule method can then do some work asynchronously and either resolve or reject the promise at some point in time.
**Note:** I stacked a diff for error handling on top of this one.
Reviewed By: mdvacca
Differential Revision: D13653156
fbshipit-source-id: 4c30c3223ad8f47c6ba7f1236527aaced01c8ae8
Summary: If some class or category override/implement one of those methods, it implies that subclasses must call super for them. `NS_REQUIRES_SUPER` allows to enforce this constraint.
Reviewed By: JoshuaGross
Differential Revision: D14896804
fbshipit-source-id: a481f16e1f7ab901d70deb6486f2ca1cf19dd8d7
Valentin Shergin
Mehdi Mulani
David Aurelio
Orta Therox
Pavel Rotek
Jonny Burger
James Ide
Diego Sanchez
Kudo Chien
Mike Diarmid
Héctor Ramos
Dulmandakh
Ramanpreet Nara
Will Holen
Kevin Gozali
Joshua Gross
Sebastian McKenzie
Jérémy M
Emily Janzer
zhongwuzw
Marc Horowitz
Spencer Ahrens
Radovan Šmitala
Rick Hanlon
Eli White
Estevão Lucas
Frieder Bluemle
Sam Mathias Weggersen
cpojer
James Treanor
Rick Ratmansky
Samuel Susla
