Summary:
Hard-coded to false everywhere, and write-only. We never read from this.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20788252
fbshipit-source-id: ae117ebc51db7045947b9713602527ff4220833e
Summary:
Remove unused feature flag. This is not used within Facebook and I'm not aware of usage outside of FB.
Changelog: [Removed] Removing Android feature flag: lazilyLoadViewManagers
Reviewed By: mdvacca
Differential Revision: D20788210
fbshipit-source-id: 435316e3de7830d7cb7f14537351883e4fc6eeaa
Summary:
## Overview
This diff is an RFC to port a logging feature from iOS to Android.
Changelog: [Internal]
## Motivation
On iOS we have the following log functions and behaviors available for logging native warnings and errors:
- **Warnings** (`RCTLogWarn`)
- Log level 'warn' to console
- Display warning in LogBox
- **Errors** (`RCTLogError`)
- Log level 'error' to console
- Display a native RedBox (needs converted to show a LogBox if available)
- **Logs**
- We also have `RCTLog`, `RCTTrace`, `RCTAdvice`, `RCTInfo`, which just log to the console.
In Java, we have:
- **Warnings**
- **None**, added in this diff
- **Errors** (`DevSupportManager.showNewJavaError`)
- Log level 'error' to console with `FLog.e`
- Display a native RedBox (needs converted to show a LogBox if available
- **Logs**
- `ReactSoftException` (crashes the app??)
- `ReactNoCrashSoftException` (only logs??)
- Others?
## Details
This diff adds a method to pair with `RCTLogWarn`, `DevSupportManager.showNewJavaWarning`, which will log to the console and show a LogBox warning if LogBox is available.
## Concerns
I have a few concerns/questions about the state of logging on Android:
- Should/can we move all of the logging to it's own class, like how RCTLog works?
- Why does some logging happen on DevSupportManager and some in other classes?
- If we moved it all to it's own class, how could we access the reactContext to call the RCTLog JS module
Reviewed By: JoshuaGross
Differential Revision: D20056394
fbshipit-source-id: 32d57e300685e46da8039fc77cb22b4084acf81a
Summary:
Get jest tests to be runnable on windows, and match current snapshots
## Changelog
[Internal] [Fixed] - More consistent snapshots on windows
Pull Request resolved: https://github.com/facebook/react-native/pull/28482
Test Plan: run `yarn test` on a windows machine, and hit the test_windows circleci tests
Reviewed By: hramos
Differential Revision: D20799002
Pulled By: cpojer
fbshipit-source-id: da3db0171c34a43199c7d3dc17b622b37bc91701
Summary:
While resolving the flexible items we calculate totalFlexShrinkScaledFactors which uses the flexBasis or initial width of node (Not min-width).
At a later stage during distribution of space we are subtracting value from this which also takes care of min-width.
For example
If node has flexShrink 1 and width 100 and min-width 301 then totalFlexShrinkScaledFactors will become -1*100 = -100
but later we are subtracting -1 * 301 (min-width) = -301 which is ambiguous and causing layout inconsistencies with how web behaves.
Fixed this by only using the flexBasis or width for these calculations.
Changelog:
[Internal][Yoga] Fix layout issue when flexShrink and min-width are used together
Reviewed By: pasqualeanatriello
Differential Revision: D20219419
fbshipit-source-id: 948fbc06ca541d4ad307c88c8a2df65d157778b1
Summary:
If name is passed in as part of RuntimeConfig, that is included
in the description, too.
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D20716320
fbshipit-source-id: f2fba6df32f496090dee787d8b7f55a6a4dd8ed8
Summary:
## Context
For now, assume TurboModules doesn't exist.
**What happens when we call an async NativeModule method?**
Everytime JS calls an async NativeModule method, we don't immediately execute it. The legacy infra pushes the call into some queue managed by `MessageQueue.js`. This queue is "flushed" or "emptied" by the following events:
- **Flushed:** A C++ -> JS call. NativeModule async methods can called with an `onSuccess` and/or `onFail` callback(s). Calling `NativeToJsBridge::invokeCallback` to invoke one of these callbacks is one way for ObjC++/C++/Java to call into JS. Another way is via JSModule method calls, which are initiated by `NativeToJsBridge::callFunction`.
- **Flushed:** When `JSIExecutor::flush` is called. Since TurboModules don't exist, this only happens when we call `JSIExecutor::loadApplicationScript`.
- **Emptied:** When more than 5 ms have passed, and the queue hasn't been flushed/emptied, on the next async NativeModule method call, we add to the queue. Afterwards, we empty it, and invoke all the NativeModule method calls.
**So, what's the difference between flushed and emptied?**
> Note: These are two terms I just made up, but the distinction is important.
If the queue was "flushed", and it contained at least one NativeModule method call, `JsToNativeBridge` dispatches the `onBatchComplete` event. On Android, the UIManager module is the only module that listens to this event. This `onBatchComplete` event doesn't fire if the queue was "emptied".
**Why does any of this matter?**
1. TurboModules exist.
2. We need the TurboModules infra to have `JsToNativeBridge` dispatch `onBatchComplete`, which depends on:
- **Problem 1:** The queue being flushed on calls into JS from Java/C++/ObjC++.
- **Problem 2:** There being queued up NativeModule async method calls when the queue is flushed.
In D14656466, fkgozali fixed Problem 1 by making every C++/Java/Obj -> JS call from TurboModules also execute `JSIExecutor::flush()`. This means that, with TurboModules, we flush the NativeModule async method call queue as often as we do without TurboModules. So far, so good. However, we still have one big problem: As we convert more NativeModules to TurboModules, the average size of the queue of NativeModule method calls will become smaller and smaller, because more NativeModule method calls will be TurboModule method calls. This queue will more often be empty than not. Therefore, we'll end up dispatching the `onBatchComplete` event less often with TurboModules enabled. So, somehow, when we're about to flush the NativeModule method call queue, we need `JsToNativeBridge` to understand that we've executed TurboModule method calls in the batch. These calls would have normally been queued, which would have led the queue size to be non-zero. So if, during a batch, some TurboModule async method calls were executed, `JsToNativeBridge` should dispatch `onBatchComplete`.
**So, what does this diff do?**
1. Make `Instance` responsible for creating the JS `CallInvoker`.
2. Make `NativeToJsBridge` responsible for creating the native `CallInvoker`. `Instance` calls into `NativeToJsBridge` to get the native `CallInvoker`.
3. Hook up `CatalystInstanceImpl`, the Android bridge, with the new JS `CallInvoker`, and the new native `CallInvoker`. This fixes `onBatchComplete` on Android. iOS work is pending.
Changelog:
[Android][Fixed] - Ensure `onBatchComplete` is dispatched correctly with TurboModules
Reviewed By: mdvacca
Differential Revision: D20717931
fbshipit-source-id: bc3ccbd6c135b7f084edbc6ddb4d1e3c0c7e0875
Summary:
Changelog: [Internal]
View with `ShadowColor` was getting flattened and therefore views didn't have shadow property set.
This is fixed by promoting ShadowColor so in case it is set, it forms stacking context.
Reviewed By: shergin
Differential Revision: D20792201
fbshipit-source-id: 1033ac00e32047ffbb14e61b7c26348c578d132d
Summary:
The next version of Babel changes how it prints file names in errors. This diff fixes the test by using `/` as the `cwd` and switches the plagin to use `path.buildCodeFrameError` so errors will be more helpful for users.
I renamed the `nodePath` variable to `path` because that's what babel plugins usually do.
Changelog: [Internal]
Reviewed By: motiz88
Differential Revision: D20781805
fbshipit-source-id: cc149dce6389aa9402ce70ea30035c74a6150ea3
Summary:
The differ was still producing correct, but not minimal, instruction sets in some cases due to an optimization that was buggy.
This affected cases where 2+ nodes were inserted at the beginning of a list. It would trigger the old behavior where all nodes after the first would be removed, deleted, then reinserted.
See the test case (which was failing and now passed) and P128190998 (the 3->4 transition) for samples.
Changelog: [Internal] Fabric differ
Reviewed By: mdvacca
Differential Revision: D20785729
fbshipit-source-id: 2fea6a816753066abb358ed7bb51003140cd5fc4
Summary:
Updating the eslint config and metro-preset used in project template.
## Changelog
[General] [Changed] - Upgrade eslint-config and metro-preset in project template
Pull Request resolved: https://github.com/facebook/react-native/pull/28443
Test Plan:
- Start new project with `npx react-native init TestLint`
- upgrade lint and metro-config
- run lint and start up emulator on iOS and android
Reviewed By: cpojer
Differential Revision: D20771048
Pulled By: hramos
fbshipit-source-id: a6d387b8687cee348681bcb10d22c7e3de291ed7
Summary:
This diff implements the instruction pointer save/restore trick Tzvetan came up with; allowing us to observe and modify the IP from outside the interpreter loop with negligible overhead.
From Tzvetan's internal post on the subject:
> [Today] the interpreter IP is just a local variable in the interpreter function, so there is no way to get to its value from outside the function. It lives in a register and we don't want to make it a Runtime field since the overhead [of accessing it via memory in the interpeter loop] would kill us.
> However, if you really think about it, it only lives in a register while the interpreter function is running. For the rest of the time, it is spilled by the C++ compiler onto the stack.
So, precisely when we need it, it is actually stored in memory. The only problem is, we don't know where! Admittedly, that is an annoying problem, but it feels like it should be solvable.
> What if, instead of relying on the compiler to spill the IP register, we manually spill it ourselves, to a known location? It works. Example: https://godbolt.org/z/ftSDnp
This diff implements this approach across the whole interpreter loop: whenever we call out of the loop we capture/publish the IP and restore it again immediately after the external call returns. This means we can now see the IP outside the interpret loop and even change it. This is effectively "for free" as the compiler now skips spilling/restoring the IP behind the scenes.
The immediate benefit of this is knowing the current IP allows us to have more accurate stack-traces during execution. In future this may enabled tricks like changing the IP before returning to the interpreter loop, allowing things outside the interpreter to affect program flow without adding logic to the interpreter loop.
Reviewed By: tmikov
Differential Revision: D20151091
fbshipit-source-id: 3814382639800208d8985a32ede31ba8f7ff7c80
Summary:
Changelog: [Internal]
`orderIndex_` was only being assigned for `ViewShadowNode`, not for other `ShadowNodes` that are later represented on the screen.
Reviewed By: shergin
Differential Revision: D20746477
fbshipit-source-id: c04c2cfea14b9141d22bc3d9e9bb4c0c59925754
Summary:
Fixes `test_android` and `test_docker` build failures. Thanks to dulmandakh for identifying the fix.
Changelog:
[Internal] [Android] [Changed] - Use Android SDK 29 to build during CI tests
Pull Request resolved: https://github.com/facebook/react-native/pull/28455
Test Plan: Circle CI shows `test_android` and `test_docker` passing: https://app.circleci.com/jobs/github/facebook/react-native/142273
Reviewed By: sturmen
Differential Revision: D20766589
Pulled By: hramos
fbshipit-source-id: 8ef8a8ce3a6e7353ae47425accb3bd26cf1608c4
Summary:
motivation is following - the RNTester app is imho the best place to try out any changes that a contributor would make, yet it is not directly mentioned in the contributor guide. This fixes it.
## Changelog
[Internal] - Docs
Pull Request resolved: https://github.com/facebook/react-native/pull/28042
Test Plan: not necessary
Reviewed By: TheSavior
Differential Revision: D20401260
Pulled By: hramos
fbshipit-source-id: 01c1b7dff56b59909c94b2feb609650f0baba1a9
Summary:
Changelog: [internal]
`ModalHostViewShadowNode` didn't have `RootNodeKit` trait, therefore `getRelativeLayoutMetrics` was including nodes in ancestors that it shouldn't have.
Reviewed By: shergin
Differential Revision: D20735801
fbshipit-source-id: 6b81e3b174c2f82e530abc2bca2da8bebc2270b0
Summary:
We are replacing inline-ed implementation with practically the same one implemented as the helper method.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20551409
fbshipit-source-id: fcc6f497cd240af65fba534051c217fe5746ce82
Summary:
Here we implement a bunch of helper methods that allow customizing the behavior of a RuntimeExecutor "on-demand" on the caller side. We will use it in the next diff(s).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: PeteTheHeat
Differential Revision: D20551411
fbshipit-source-id: 51d3cd02b69753110c0e1155347c6e52eb882c7d
Summary:
Refs facebook/react-native-website#1776.
Despite in-code description `PixelRatio.getFontScale()` is working properly on the iOS (it also reflects the user settings).
This PR updates the in-code description to match current behaviour.
I have decided to skip in the code information about additional setting in `Accessibility` menu and in `Control Centre`, but if you think it is important just let me know, I can update this PR.
## Changelog
[Internal] [Fixed] - Fix PixelRatio getFontScale method description
Pull Request resolved: https://github.com/facebook/react-native/pull/28407
Test Plan: N/A
Differential Revision: D20750260
Pulled By: shergin
fbshipit-source-id: c40ec2fd49cd60e2975351c3a1c453aab0045da4
Summary:
Some of the repository name of Lean Core(https://github.com/facebook/react-native/issues/23313) libraries has been renamed.
This PR updates the warning message to display the updated url.
## Changelog
[General] [Changed] - Update warning message of deprecated imports
Pull Request resolved: https://github.com/facebook/react-native/pull/28452
Test Plan: updated URL can be accessed.
Reviewed By: cpojer
Differential Revision: D20745184
Pulled By: TheSavior
fbshipit-source-id: 2c3ed6a000b45022ca6c4862305aa567c4d18b2e
Summary:
## Overview
This diff refactors the Inspector, moving logic to look up view data for a touched view inside the renderer as `getInspectorDataForViewAtPoint`. We then implement that same function for Fabric in order to support the inspector in that renderer.
Requires https://github.com/facebook/react/pull/18388
## Motivation
Reason one for this refactor is that, previously, the inspector held all of the logic to look up view data for a given x,y touch coordinate. To do this, it would take the React tag and coordinates, look up the native view tag, measure it, and then ask React internals for the Fiber information of that tag. All of this is deeply coupled to React internals, yet the logic is outside of React core in the Inspector.
Reason two is that, for Fabric, the logic for getting the view data is different than Paper. In Fabric, we pass the x,y coordinates to native directly, which returns an instance handle. That handle can be used to measure the ShadowNode, or retrieve the Fiber information.
By moving the logic into the renderer in React core, we decouple the implementation details of looking up view data for a tapped point and allow ourselves the ability to add and change renderer-specific code for the actual lookup without impacting outsiders like the Inspector.
Changelog: [Internal]
Reviewed By: TheSavior
Differential Revision: D20291710
fbshipit-source-id: a125223f2e44a6483120c41dc6146ad75a0e3e68
Summary:
There's (potentially) a lot of expensive reflection calls here that, as best I can tell, end up being ignored if the supplied color is null. Better to early return.
Changelog: [General] [Internal] Preclude reflection when setting cursor color if color is null
Reviewed By: JoshuaGross
Differential Revision: D20670594
fbshipit-source-id: 480a988355bbd79008002c4326d4b35035ec2a95
Summary:
Accessing this field via reflection is explicitly blacklisted by Google in Android 10 and higher. They have provided a new API to change the color, which I have implemented here. [The old setColorFilter is deprecated](https://developer.android.com/reference/android/graphics/drawable/Drawable#setColorFilter(int,%20android.graphics.PorterDuff.Mode)) so I also updated that method call as well.
Changelog: [General] [Fixed] Use new setTextCursorDrawable API for Android 10
Reviewed By: JoshuaGross
Differential Revision: D20656068
fbshipit-source-id: 58a92b57c0a892c7c87fc5d735e4ceaa4e987ec7
Summary:
This issue closes https://github.com/facebook/react-native/issues/28241
Migrated Windows test from AppVeyor to CircleCI
## Changelog
[Internal] [Changed] - Migrated Windows test from AppVeyor to CircleCI
Pull Request resolved: https://github.com/facebook/react-native/pull/28245
Test Plan: For CircleCI to Pass
Reviewed By: cpojer
Differential Revision: D20689163
Pulled By: hramos
fbshipit-source-id: 285c762457ef00f7884ee9157b3f336044c0452f
Summary:
Changelog: [Internal]
Current implementation of `measure` doesn't take transform into account..
So if you had a view which has width and height 100 and had `Scale(0.5, 0.5, 1)` (this will shrink view by half). Calling `getRelativeLayoutMetrics` would report its size being `{100, 100}`.
This applies if view's parent has transformation as well, because transformation is applied to all subviews of the view as well.
Reviewed By: mdvacca
Differential Revision: D20621590
fbshipit-source-id: 2cf902a0494291c821ecada56f810c5e6620db5a
Summary:
Changelog: [Internal]
Until now `Rect operator*(Rect const &rect, Transform const &transform)` supported only scaling. Now it supports translation and rotation as well.
Reviewed By: shergin
Differential Revision: D20622876
fbshipit-source-id: 1b65393bd3fd6fd9a8941903e0f2681a10097e4a
Summary:
`LayoutInspectingPolicy` has two flags, `includeTransform` and `includeScrollViewContentOffset`.
`includeScrollViewContentOffset` seems to be redundant for two reasons.
# 1st
From looking at callers, they have always the same value.
I looked at all call sites, and they are either always both set to true or both set to false.
# 2nd
The way we include scroll view content offset, is through transformation, so setting `includeTransform` to true and `includeScrollViewContentOffset` to false will include content offset anyway. In order to make both flags work, we would need to introduce further changes to `getRelativeLayoutMetrics`. But since the flag isn't used anyway, I think it is better to get rid of it for now. If we need it in the future, we could re-introduce it.
Reviewed By: shergin
Differential Revision: D20622256
fbshipit-source-id: fb6156c66b752319ea928239fa723ff90688b0a0
Summary:
Two things:
1) I reimplement Valentin's idea in D19965405, so that TinyMaps can be iterated over, with a couple of bugfixes (calling front() or back() on an empty vector will crash).
2) I now use TinyMap instead of better::map in the "optimized" diffing algorithm.
3) `erase` now actually removes elements from the vector, but only when more than half of elements have been erased.
4) If you repeatedly erase elements at the beginning of the vector, they will no longer be iterated over. This is a specific optimization for our heaviest TinyMap use-cases.
These amount to some small but hopefully somewhat meaningful perf improvements.
Changelog: [Internal] Fabric perf
Reviewed By: shergin
Differential Revision: D20718719
fbshipit-source-id: 91f4b2e2e0f6387ae484e43d5b0095103087baa6
Summary:
This fix aims to address the issue when bundling an Android app for release and getting the error exhibited in https://github.com/facebook/react-native/issues/28002 which I also encountered myself.
The config was changed sometime in November 2019 (as part of https://github.com/facebook/react-native/issues/26940, commit a3b0804867) to be very opinionated when it comes to the use of `npx` which Gradle itself cannot find anyway (I have `npx` installed globally and it didn't pick it up).
Another issue that the use of `npx` creates is that Gradle should only ever use the currently installed react-native cli rather than a (possibly) higher version which may not always have backward compatibility.
The proposed change simply throws a more descriptive error rather than defaulting to a tool which may or may not exist on the machine, be it CI or a development environment. I've also modified the RNTester app to reflect the correct config implementation relative to the RNTester app itself.
In real projects, the config inside `android/app/build.gradle` should look similar to the following snippet:
```
project.ext.react = [
cliPath: "$rootDir/../node_modules/react-native/cli.js",
entryFile: "index.js"
];
```
## Changelog
[Android] [Fixed] - Gradle release config
Pull Request resolved: https://github.com/facebook/react-native/pull/28415
Test Plan:
- [x] Successfully bundled an Android release build with correct config
- [x] Works with RNTester app
Reviewed By: mdvacca
Differential Revision: D20714372
Pulled By: hramos
fbshipit-source-id: 4d66139249c6f840582a71a48c64e6a6595f7af0
Summary:
The [PlatformColor PR](https://github.com/facebook/react-native/pull/27908) added support for iOS and Android to express platform specific color values. The primary method for an app to specify such colors is via the `PlatformColor()` method that takes string arguments. The `PlatformColor` method returns an opaque Flow type enforcing that apps use the PlatformColor method instead of creating Objects from scratch -- doing so would make it harder to write static analysis tools around Color values in the future. But in addition to `PlatformColor()`, iOS has a `DynamicColorIOS()` method that takes an Object. The Flow type for this Object cannot be opaque, but we still want to enforce that app code doesn't pass variables instead of Object literals or that values in the Objects are variables. To ensure `DynamicColorIOS()` can be statically analyzed this change adds an ESLint rule to enforce that `DynamicColorIOS()` takes an Object literal of a specific shape. A `ColorAndroid()` was also introduced not for practical use but just to test having platform specific methods for more than one platform in the same app. A second ESLint rule is created for `ColorAndroid` as well.
## Changelog
[General] [Changed] - Add ES Lint rules for `DynamicColorIOS()`and `ColorAndroid()`
Pull Request resolved: https://github.com/facebook/react-native/pull/28398
Test Plan: `yarn lint` passes.
Reviewed By: cpojer
Differential Revision: D20685383
Pulled By: TheSavior
fbshipit-source-id: 9bb37ccc059e74282b119577df0ced63cb9b1f53
Summary:
This unit test is to verify that the new diffing algorithm generates a "minimal" instruction set, with regards to removes and inserts ("moves").
These unit tests are here to verify the expected behavior in this new algorithm, but these tests may be modified or deleted in the future if we decide we want to change this behavior.
Changelog: [Internal] fabric unit test
Reviewed By: mdvacca
Differential Revision: D20706592
fbshipit-source-id: 5f9991498e0d788ecbf88d938bfe6d3f0f27af40
Summary:
An evolution of D20633188 but more performant.
There are three optimized paths before the slow path.
The first optimized path tries to pair identical nodes from old/new tree, and generate Update mutations, until we hit nodes that are different (indicating either a remove or an insert). This already existed.
The next two optimizations, introduced by Tim in his JS pseudocode, were inspired by ReactJS's diffing algorithm. They work in cases where the rest of the nodes are (1) all removals/deletes or (2) all creates+inserts.
Finally, if those final two optimized paths can't run, it's because there is a mix of delete+remove, create+insert, and "move" operations, mixed at the beginning, middle, and/or end of the list.
This has slightly better average/best-case complexity as the previous implementation.
In particularly pathological cases where all nodes are arbitrarily reordered, or reversed, for instance (ABCDE->EDCBA) the algorithm has the same complexity as the previous algorithm (quadratic).
For now iOS is pinned to the older differ
Changelog: [Internal] Experiment to optimize diffing algorithm in Fabric
Reviewed By: shergin
Differential Revision: D20684094
fbshipit-source-id: d29fba95a0328156c023e1c87804f23770ee1d91
Summary:
`std::this_thread::sleep_for` is not really precise and will attempt to sleep for "at least" that much time, but may sleep much longer depending on what CPUs are doing and scheduling policies.
To get this to pass on my machine, I had to substantially increase the thresholds.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D20689571
fbshipit-source-id: f159420d24a95da2b5d95d49ed7882e783291e98
Summary:
State Reconciliation has been running 50/50 for a while and all metrics look stable. This is necessary for providing a good experience so we should ship to everyone unconditionally.
Changelog: [Internal] Fabric diffing reconciliation process improvement
Reviewed By: mdvacca
Differential Revision: D20715694
fbshipit-source-id: 25b2635ecc29b67e2911679c9db66bc84d37dec1
Summary:
In preparation for upgrading babel, I'm updating some of our source transform tools to the latest versions.
Changelog: [Internal]
Reviewed By: GijsWeterings
Differential Revision: D20675201
fbshipit-source-id: fa4fee2652529c6677087e42cdd1325a8080e46f
Joshua Gross
Rick Hanlon
Andrew Coates (REDMOND)
Sidharth Guglani
Marc Horowitz
Ramanpreet Nara
Samuel Susla
Christoph Nakazawa
Héctor Ramos
Chatura Atapattu
Jesse Katsumata
Jacob Bower
Emilis Baliukonis
Vojtech Novak
Valentin Shergin
Bartosz Kaszubowski
Lucas Bento
Nicholas Tinsley
Spencer Ahrens
Ventsislav Dimitrov
Tom Underhill
