[pull] master from microsoft:master#8
Merged
Saadnajmi merged 719 commits intoSaadnajmi:masterfrom Sep 3, 2021
Merged
Conversation
Summary: See title. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21965838 fbshipit-source-id: a3bd9d48c1fadb19a12cccaab5713cf73566bd93
…k#29090) Summary: Pull Request resolved: facebook#29090 Moving the logic for calling into JS to handle errors into ErrorUtils, where it can be reused outside of the bridge. Reviewed By: RSNara Differential Revision: D21939254 fbshipit-source-id: 0d8f3bd2503720be7619ed8dc8b2389f544049f3
Summary: This diff exposes receiveEvent on the UIManager class. This is necessary to support backward compatibility between Fabric and classic RN changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D21979687 fbshipit-source-id: 1ec75896687d55e699f79c520e21f05fac368ee6
Summary: This diff migrates usages of RCTEventEmitter.class to EventDispatcher.dispatchEvent. RCTEventEmitter is not compatible with Fabric, now we need to use EventDispatcher.dispatchEvent instead. changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D21982549 fbshipit-source-id: 9ea2d9a00f063a03c2e401fc1e328ce26bcf48df
…EventEmitter Summary: This diff refactors ReactTextView to use uiManager.receiveEvent instead of ReactEventEmitter. ReactEventEmitter.class should be replaced for uiManager.receiveEvent. changelog: [internal] Reviewed By: JoshuaGross Differential Revision: D21982548 fbshipit-source-id: 4ed5825f62c761564aa533f4e8bb1155036df7e2
Summary: Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D21995098 fbshipit-source-id: b387bd45b44fe93743d34d2a3435c165d15f054f
Summary: Changelog: [internal] We were not reseting value of slider when recycling, this would result in wrong value being displayed once slider gets reused. Reviewed By: shergin Differential Revision: D21996786 fbshipit-source-id: 3beac4936d0719c4ac5ac0499209300a912f6986
Summary:
Changelog: [Internal]
Here is what I believe happens.
We have an instance of `RCTSliderComponentView` which has green thumb tint color.
It gets reused, in prepareForRecycle we call `setThumbTintImage:nil` on its UISlider which internally sets an ivar to `nil`.
Next time `RCTSliderComponentView` gets used without explicit thumb tint color, we assign nil to UISlider's thumb tint color.
Internally this nil gets compared to nil that it saved during `prepareForRecycle` and concludes that the value is already sets and exists early.
Since we don't have access to `UISlider` I can't prove this but here is a short video where I showcase this behavior
{F239923204}
The code in video is here P133083862.
Reviewed By: shergin
Differential Revision: D21997324
fbshipit-source-id: 28a11ed817cc863a313217c475042918ee726011
Summary: Changelog: [Internal] There is no need for custom dealloc, let's get rid of it. We also prefer east const over west const. Reviewed By: shergin Differential Revision: D21997545 fbshipit-source-id: aa017f99aa26421fc59b353d0012687cb38fac08
…wNodeFragment::Value Summary: There is no good reason why they are `const` and `private`, in the future diffs we will need to mutate them. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D21992197 fbshipit-source-id: f8aee8db9ea1ff8282b38fbe3a966911678ee2c4
Summary: Changelog: [Internal] Round calculated text size to closest larger pixel size. In Paper we do this as well in https://fburl.com/diffusion/w2pj6ea0 Why do we need this? For example, we calculate height of the text to be 16.707235, yoga takes this value and assigns text to have height 17 anyway. I assume Yoga uses this extra 0.3 points of height to move other things around a little bit because Yoga doesn't deal with exact values. Reviewed By: shergin Differential Revision: D21999032 fbshipit-source-id: 73923dc3e27fa110adb3f76cfa635e0bfdc672d4
Summary: Fixes some comment typos, moves hit testing and accessibility code so it's beside each other. No functionality changes. Changelog:[Internal] Reviewed By: RSNara Differential Revision: D22003047 fbshipit-source-id: 0e785364d7e1a080034d24c9676a56acb45686bb
Summary: The initialScrollIndex in VirtualizedList contains a performance optimization to start rendering the list at the index provided. ContentOffset does not contain this optimization and there is currently no way to specify the first item in the list to start rendering without contentOffset being ignored. This change makes it so that if both initialScrollIndex and ContentOffset are provided, the list will start rendering at the initialScrollIndex but ContentOffset will still be used to set the scroll position. initialScrollIndex functionality will remain the same if ContentOffset is not provided. Changelog: [Changed] VirtualizedList will use contentOffset for scroll position instead of initialScrollIndex if both are provided Reviewed By: sahrens Differential Revision: D21980172 fbshipit-source-id: 36d2d2bc360845ef02329d2b95a2cf14b91c2b0b
…c or Paper lifecycle Summary: Switch between "Fabric" and "Non-Fabric" modes based on which types of native Views are being attached to animations. Don't allow non-Fabric to drive Fabric animations and vice-versa. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21985411 fbshipit-source-id: fb9bef1e38375b384430b4e0275e7b6d62eda7a4
… zero Summary: Fabric will crash if Inf/NaN values are passed to the native side, so just pass zero to native in those cases. It's an equally (in)valid value and we're already logging the error. Also, debounce the error so we don't spam the LogBox/console. This should potentially be changed to a warning. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22008980 fbshipit-source-id: 1aef33a13febed04311557718f05ef79bbbcca93
Summary: see title Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22008982 fbshipit-source-id: 056b0d99e8e81a1b11cd054e6c813002ae0b7014
…tchMountItem Summary: In particular, NativeAnimatedModule relies on having some signal of when there's a commit from ReactJS. In Fabric, this is the only reliable signal. Failing to call scheduleMountItems even when there's no changes to the tree will result in certain animation operations being delayed way longer than necessary. I pass nullptr instead of empty arrays in some cases to hopefully improve perf. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22008981 fbshipit-source-id: 6bdeb46e03b5138dbfa5b077952ec0fa3dfe1eb3
Summary: This pull request updates the Podspecs to reflect recent changes to Fabric, so they build with Fabric enabled again. In `React-Fabric.podspec`, subspecs for the views `legacyviewmanagerinterop`, `safeareaview`, `textinput`, and `unimplementedview` are added. A minor tweak to the subspec for `rncore` was added blacklisting two codegen-generated files that have issues building, but are not actually needed yet and are empty. In `React-graphics.podspec`, the header paths for Boost were added. This is partially a retread of facebook#26805, which had to be backed out due to issues with the changes to the Yoga podspec. ## Changelog [Internal] [Fixed] - Fixed building Fabric with the Cocoapods Pull Request resolved: facebook#28584 Test Plan: RNTester can now compile. Reviewed By: shergin Differential Revision: D20966065 Pulled By: hramos fbshipit-source-id: 73931be63b72fcb52705643f81c0f787e571eb76
Summary:
Changelog: [internal]
Minimal implementation of `accessibilityState` that supports `disabled` and `selected` props.
Complete implementation will need to support following props
```
export type AccessibilityState = {
disabled?: boolean,
selected?: boolean,
checked?: ?boolean | 'mixed',
busy?: boolean,
expanded?: boolean,
...
};
```
Reviewed By: mdvacca
Differential Revision: D22016743
fbshipit-source-id: 1748acc4279f60ec8a92c93d5d13b136f57eb8d3
Summary: telling native that `selected` is the new selection is basically ignoring the most recent user input and resetting it back to the previous value. the reason it flips back and forth is that by the next time `onSelect` is called, the above code has updated the value of `selected` (line 63) Changelog: [Android] [Fixed] Picker - fix usage of setNativeSelectedPosition in onSelect Reviewed By: mdvacca Differential Revision: D22022456 fbshipit-source-id: fe51f9cbd712b5578e9fd9ea2992bd12d48f4ec4
Summary: Changelog: [Internal] Prop autoFocus was not implemented in Fabric's TextInput. Reviewed By: mdvacca Differential Revision: D22019333 fbshipit-source-id: 03f043b93e1079a5d0bff55b08ebc9d2f973c55b
…eliable Summary: Similar to D21756178, we cannot rely on childCount since it can return zero when there are actually valid children. This both causes more exceptions than necessary when the operation would work, and pollutes error messages since the information is not strictly reliable. Instead, we just try to get a View and thrown an exception when it's null, or in loops, loop until we hit a null child. `getChildAt` doesn't throw exceptions, it just returns null when we're out-of-bounds. This can impact custom ViewGroups like BottomSheets, and other ViewGroups that might do interesting/weird things with children, including ReactClippingViewManager. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22035569 fbshipit-source-id: 43e98d81178faaf720face98fc84e78743f292c3
Summary: This diff changes the order of execution for the "updatePadding" mount item Padding mountItems must be executed before layout props are updated in the view. This is necessary to ensure that events (resulting from layout changes) are dispatched with the correct padding information. This fixes a 'flickering' bug in Marketplace Vehicles (see test plan) changelog:[internal] Reviewed By: JoshuaGross, sammy-SC Differential Revision: D22034850 fbshipit-source-id: 222fa9412dd01f65a1a034f53e1eb0e7b774ec1f
Summary: This diff fixes a race condition that caused a flicker during initial rendering of TextInput in Fabric The root cause is that the TextInput's State is sometimes initialized with no information from the Theme, this causes text input to be rendered with 0 padding. Later ReactTextInput manager updates TextInput state with theme data and the TextInput is re-rendered with the correct padding information. changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D22034849 fbshipit-source-id: a2fa91f34a8340878f2ec8d231ef6c86cee08f05
Summary: Changelog: [Internal] Reviewed By: javache Differential Revision: D21206851 fbshipit-source-id: 67ab59688c19870ef419711fdfd489bf0442bb54
Summary: Fix crashes in LayoutAnimations: 1) It is valid to omit create, update, delete configs 2) When extracting SRT from a matrix, ignoring skew properties 3) Provide valid telemetry from LayoutAnimations transactions Unrelated to crashes: to help debugging and until onSuccess/onError callbacks are working, log any configuration parsing errors. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22050736 fbshipit-source-id: e59418ecad0f9bfd20a2b4976557e39020c2d101
Summary: Since we were not busting the `surfaceActiveForExecution` cache, it is very possible (likely, even!) that a surfaceId is invalidated in between UI ticks but we keep executing items for that surface at the next tick. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22055303 fbshipit-source-id: 351c13535e85b30e00684fe35fc4aa79ccb5961c
Summary: Gate stopSurface behind a feature flag. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22054480 fbshipit-source-id: 3ea48ab46aeb3532fc7a3dd83659d7c32891ec06
Summary: Changelog: [Internal] A long time ago we experimented with JSC bytecode. We are not experimenting with JSC bytecode any more. This code can be removed. Reviewed By: mhorowitz Differential Revision: D22017374 fbshipit-source-id: 6fe3fb7ad7966f92a5cd103605ac5c0bd1f17a8e
…an/0.64-merge-2020-mm-dd
Summary: Changelog: [internal] Revert a change introduced in facebook#30333 where rickhanlonii asked it to be reverted as well. The change breaks sticky header in Fabric. Reviewed By: rubennorte Differential Revision: D25883861 fbshipit-source-id: b01305c6def390d0664c7be939ab3fc72186a07a
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
Merge from upstream through 2020-06-07
…an/0.64-merge-2020-mm-dd
Fix Android Build Issues for RN64 Upgrade for patches through 2020-06-30
Merge from upstream through 2020-06-30
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
Summary: It is now required to call RCTBridge.setRCTTurboModuleRegistry for turbo modules to work properly on iOS. ## Changelog [Internal] [Fix] - Fix RNTester TurboModules loading Pull Request resolved: facebook#29538 Test Plan: Tested that images in RNTester now loads properly and Redbox module missing warning are gone. Reviewed By: JoshuaGross Differential Revision: D22884163 Pulled By: fkgozali fbshipit-source-id: daf2fccdb518ec4a382b80b7f7b02004405a7564
…an/0.64-merge-head
…nputShouldReturn]
Merge from upstream through 2020-07-17
Saadnajmi
pushed a commit
that referenced
this pull request
Oct 21, 2021
Summary:
changelog: [internal]
Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t
# Why is using size_t better when working with indexes.
## 1. Type conversion isn't for free.
Take this example
```
size_t calculate(int number) {
return number + 1;
}
```
It generates following assembly (generated with armv8-a clang 10.0.0):
```
calculate(int): // calculate(int)
sub sp, sp, #16 // =16
str w0, [sp, #12]
ldr w8, [sp, #12]
add w9, w8, #1 // =1
mov w8, w9
sxtw x0, w8
add sp, sp, #16 // =16
ret
```
That's 9 instructions.
If we get rid of type conversion:
```
size_t calculate(size_t number) {
return number + 1;
}
```
Assembly (generated with armv8-a clang 10.0.0):
```
calculate(unsigned long): // calculate(unsigned long)
sub sp, sp, #16 // =16
str x0, [sp, #8]
ldr x8, [sp, #8]
add x0, x8, #1 // =1
add sp, sp, #16 // =16
ret
```
Compiler now produces only 7 instructions.
## Semantics
When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.
Reviewed By: JoshuaGross
Differential Revision: D24332248
fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )