Commit Graph

907 Commits

Author SHA1 Message Date
Ruslan Shestopalyuk a142a78473 Fix regression when setting shadow node properties in D38272966 (github PR merge)
Summary:
Changelog:

[Android][Fixed] - Fix regression when setting shadow node properties.

Also simplified the corresponding macros to avoid using lambdas altogether, as they are not required.

Note that this **does not** modify any constexpr-related semantics of the existing code, as the main constexpr macro, `CONSTEXPR_RAW_PROPS_KEY_HASH` evaluation result is still contstexpr value, and the other ones already involved non-const parts (see my comments).

Reviewed By: NickGerleman

Differential Revision: D38356411

fbshipit-source-id: 22c330d3425c8aed36693f4652f1b257d2dc96be
2022-08-02 15:14:16 -07:00
Dark Knight 18b6e53c0b Revert D38280674: Multisect successfully blamed D38280674 for test or build failures
Summary:
This diff is reverting D38280674 (https://github.com/facebook/react-native/commit/a561df0e96da334a44e4e8b798c1be9e19fa1bc2)
D38280674 (https://github.com/facebook/react-native/commit/a561df0e96da334a44e4e8b798c1be9e19fa1bc2) has been identified to be causing the following test or build failures:
Tests affected:
- https://www.internalfb.com/intern/test/562950020230233/

Here's the Multisect link:
https://www.internalfb.com/intern/testinfra/multisect/1102425
Here are the tasks that are relevant to this breakage:
T93118125: [7 CRITICAL TEST FAILURES] 12 tests started failing for oncall marketplace_feed_experience in the last 2 weeks
We're generating a revert to back out the changes in this diff, please note the backout may land if someone accepts it.

 ---

Changelog:
[General][Changed] - Reverting "[react-native] Exit early if currentShadowNode cannot be casted" due to test failures

Differential Revision: D38338773

fbshipit-source-id: 0e0c11b1f5e27d98fdcfab34847a2fd0750ff5af
2022-08-02 00:11:58 -07:00
Chiara Mooney fc26dbfce0 Fix Macro Errors for Windows (#34299)
Summary:
Fix macro errors for Windows. Current syntax breaks the build of the React Common project on Windows because the ({...}) syntax is not supported; must be replaced with lambda expressions.

Resolves https://github.com/facebook/react-native/issues/34090

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Fixed] - Fix macro errors for Windows.

lyahdav JoshuaGross

Pull Request resolved: https://github.com/facebook/react-native/pull/34299

Test Plan: Build on react-native-windows repo. Tested in RNW app.

Reviewed By: javache

Differential Revision: D38272966

Pulled By: NickGerleman

fbshipit-source-id: e76eac11cde173ef49465d01d793c593017f2ab7
2022-08-01 20:54:59 -07:00
David Vacca a561df0e96 Exit early if currentShadowNode cannot be casted
Summary:
This diff fixes the bug T127619309 by exit early during calculateTransformedFrames if currentShadowNode cannot be casted

This is a bug that fired in fb4a but we didn't have a way to reproduce locally.
We are going to release this and enable feature flag with a MC

changelog: [internal] internal

Reviewed By: sammy-SC

Differential Revision: D38280674

fbshipit-source-id: 1c42c17678d8473564e4075a78d3c688efed1a23
2022-08-01 13:57:36 -07:00
David Vacca 1e4ebf2531 Disable calculation of TransformedFrames in Layoutable ShadowNodex
Summary:
Calculation of TransformedFrames was a method introduced by D37994809 (https://github.com/facebook/react-native/commit/64528e5faa445907b8287b412c344f30c20fca61) to fix rendering of inverted flat lists.

We found that this operation is crashing in fb4a causing the UBN T127619309

This diff creates a feature flag to disable the calculation of TransformedFrames in Layoutable ShadowNodes. **The goal of this diff is to revert the behavior introduced by D37994809 (https://github.com/facebook/react-native/commit/64528e5faa445907b8287b412c344f30c20fca61)**

The featureFlag is disabled in fb4a and enabled in react AR (because ReactAr apps relies on the calculation of TransformedFrames and these apps are not affected)

The root cause of the bug will be fixed by D38280674 (which still requires more testing and investigation)

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D38286857

fbshipit-source-id: 721cd0554ae6a6b369b3f8dbb584160a270d0f18
2022-07-29 17:32:35 -07:00
Graham Mendick e24ce708ab Migrate needsCustomLayoutForChildren check to the new architecture (#34254)
Summary:
Fixes https://github.com/facebook/react-native/issues/34120

The new React Native architecture doesn't check `needsCustomLayoutForChildren` so it wrongly positions native views on Android. In https://github.com/facebook/react-native/issues/34120 there are videos comparing the positioning of a native action view in the old and the new architecture.

This PR passes the parent tag to the `updateLayout` method of the `SurfaceMountingManager`. The `SurfaceMountingManager` calls `needsCustomLayoutForChildren` on the parent view manager (copied the code from the `NativeViewHierarchyManager` in the old architecture).

**NOTE** - I wasn't sure where to get the parent shadow view from so I've put in my best guesses where I could and left it as `{}` otherwise.

## Changelog

[Android] [Fixed] - Migrate `needsCustomLayoutForChildren` check to the new architecture

Pull Request resolved: https://github.com/facebook/react-native/pull/34254

Test Plan:
I checked the fix in the repro from https://github.com/facebook/react-native/issues/34165. Here is a video of the action view closing using the native button that is now visible in the new architecture.

https://user-images.githubusercontent.com/1761227/180607896-35bf477f-4552-4b8a-8e09-9e8c49122c0c.mov

Reviewed By: cipolleschi

Differential Revision: D38153924

Pulled By: javache

fbshipit-source-id: e2c77fa70d725a33ce73fe4a615f6d884312580c
2022-07-28 09:57:36 -07:00
Vincent Riemer 546c4b4c6c Add isPrimary property implementation to the PointerEvent object
Summary:
Changelog: [iOS][Internal] - Add isPrimary property implementation to the PointerEvent object

This diff adds the `isPrimary` property to the PointerEvent object iOS implementation. In addition this adds a related change where we "reserve" the 0 touch identifier for mouse events and the 1 identifier for apple pencil events. This is an easy way to ensure that these pointers are always consistent no matter what happens. Since mouse & pencil pointers should always be considered the primary pointer, that allows us to focus the more advanced primary pointer differentiation purely on touch events.

The logic for this touch event primary pointer differentiation is essentially setting the first touch it recieves as a primary pointer, setting it on touch registration, and sets all subsequent touchs (while the first touch is down) as not the primary pointers. When that primary pointer is lifted, the class property keeping track of the primary pointer is reset and then the **next** pointer (secondary pointers which had already started before the previous primary pointer was lifted are not "upgraded" to primary) is marked as primary. A new platform test is also included in this diff in order to verify the aforementioned behavior.

Reviewed By: lunaleaps

Differential Revision: D37961707

fbshipit-source-id: ae8b78c5bfea6902fb73094fca1552e4e648ea44
2022-07-26 11:41:48 -07:00
Tony Du 1e3cb91707 Allow multiline TextInputs be submittable without blurring (#33653)
Summary:
For multiline TextInputs, it's possible to send the submit event when pressing the return key only with `blurOnSubmit`. However, there's currently no way to do so without blurring the input and dismissing the keyboard. This problem is apparent when we want to use a TextInput to span multiple lines but still have it be submittable (but not blurrable), like one might want for a TODO list.

![multiline-momentary-blur](https://user-images.githubusercontent.com/22553678/163596940-aae779f5-4d2a-4425-8ed0-e4aa77b90699.gif)

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Added] - Add `returnKeyAction` prop to `TextInput` component
[General] [Deprecated] - Remove usages of `blurOnSubmit` in native code and convert `blurOnSubmit` to `returnKeyAction` in the JavaScript conversion layer

Pull Request resolved: https://github.com/facebook/react-native/pull/33653

Test Plan:
Verified old usages of combinations of `blurOnSubmit` and `multiline` matched previous behavior and that the new `returnKeyAction` prop behaves as expected.

| Android | iOS |
| --- | -- |
| ![android-returnkeyaction-test](https://user-images.githubusercontent.com/22553678/163597864-2e306f98-7b6e-4ddf-8a35-625d397d3dce.gif) | ![ios-returnkeyaction-test](https://user-images.githubusercontent.com/22553678/163598407-9e059f74-3549-4b46-8e03-c19bfaa6dd3d.gif)  |

With the changes, the TODO list example from before now looks like this:

![multiline-no-momentary-blur](https://user-images.githubusercontent.com/22553678/163598810-f3a71d62-5514-486e-bf6a-79169fe86378.gif)

Reviewed By: yungsters

Differential Revision: D35735249

Pulled By: makovkastar

fbshipit-source-id: 1f2237a2a5e11dd141165d7568c91c9824bd6f25
2022-07-22 13:08:45 -07:00
Eric Edouard 8993ffc82e Added border curve style prop ("Squircle" effect - iOS only) (#33783)
Summary:
<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->
NOTE: PR is based on https://github.com/facebook/react-native/pull/32017 which went stale for quite a long time but can now safely be closed

![](https://preview.redd.it/nuvl4746ys471.png?width=960&crop=smart&auto=webp&s=084a517a645364ac246b70b7fa8e0f2470cc7af3)

Since iOS 13+, it is possible to change the corner curve property on iOS in order to smoothen border radius and make it more "rounded" (also called "squircle")
Here's an [article](https://medium.com/arthurofbabylon/a-smooth-corner-radius-in-ios-54b80aa2d372) explaining in details what it is.
This property is also built in figma, but currently there is no way to implement this directly with react-native despite it being available natively on iOS.

Many open source react-native libraries were created in order to simulate this behaviour:
[react-native-super-ellipse-mask](https://github.com/everdrone/react-native-super-ellipse-mask)
[react-native-squircle-view](https://github.com/everdrone/react-native-squircle-view)
[react-native-figma-squircle](https://github.com/tienphaw/react-native-figma-squircle)

But they rely on creating an SVG shape with the smoothed corners and masking the view behind. This makes it not very performant (flickering on mounting was a common side-effect)

This PR aims at implementing the property natively.

PR for the docs update: https://github.com/facebook/react-native-website/pull/2785

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[iOS] [Added] - Added `borderCurve` style prop for smooth border radius (squircle effect)

Pull Request resolved: https://github.com/facebook/react-native/pull/33783

Test Plan:
We used the RNTester app and added an example with `cornerCurve ` set to `'continuous'` (only on iOS).

As the difference is quite subtle, we also made some more tests to better illustrate the difference (these are not in the RN-tester app):

![IMG_0810](https://user-images.githubusercontent.com/19872411/133893536-26207c53-aade-4583-9eef-7a1739b6907b.PNG)

We overlapped two views with `position: absolute`, the one in the background has a red background and has `cornerRadius` set to `false`, and the one in the foreground is set to `true`. We can clearly see where the borders differs on the corners.

Reviewed By: sammy-SC

Differential Revision: D37883631

Pulled By: cipolleschi

fbshipit-source-id: 09f06de9628fa326323eba63875de30102c4a59e
2022-07-21 04:11:30 -07:00
Samuel Susla 64528e5faa Fix measure when view is inverted
Summary:
changelog: [internal]

Vertical and horizontal inversion was not taken into account in `computeRelativeLayoutMetrics`. To fix it, we precompute frames to include inversions.

Reviewed By: mdvacca

Differential Revision: D37994809

fbshipit-source-id: 043e6f19b6fa577f61fa3c739ce2d751ef973cc3
2022-07-21 03:34:56 -07:00
Samuel Susla d0b1d49e9a Delete mobile config react_native_new_architecture:enable_call_immediates_ios
Summary:
changelog: [internal]

Remove shipped mobile config

Reviewed By: cortinico

Differential Revision: D37913209

fbshipit-source-id: 1a5877360e12e329342d4c8efc0a10bff24abcaa
2022-07-20 04:01:36 -07:00
Nick Gerleman 089c9a5c9c Fix AttributedString comparison logic for TextInput state updates
Summary:
D37801394 (https://github.com/facebook/react-native/commit/51f49ca9982f24de08f5a5654a5210e547bb5b86) attempted to fix an issue of TextInput values being dropped when an uncontrolled component is restyled, and a defaultValue is present. I had missed quite a bit of functionality, where TextInput may have child Text elements, which the native side flattens into a single AttributedString. `lastNativeValue` includes a lossy version of the flattened string  produced from the child fragments, so sending it along with the children led to duplicating of the current input on each edit, and things blow up.

With some experimentation, I found that the text-loss behavior only happens on Fabric, and is triggered by a state update rather than my original assumption of the view manager command in the `useLayoutEffect` hook. `AndroidTextInputShadowNode` will compare the current and previous flattened strings, to intentionally allow the native value to drift from the React tree if the React tree hasn't changed. This `AttributedString` comparison includes layout metrics as of D20151505 (https://github.com/facebook/react-native/commit/061f54e89086af1c80e5b0460ec715533f99bdb7) meaning a restyle may cause a state update, and clear the text.

I do not have full understanding of the flow of state updates to layout, or the underlying issue that led to the equality check including layout information (since TextMeasurementCache seems to explicitly compare LayoutMetrics). D18894538 (https://github.com/facebook/react-native/commit/254ebab1d2b6fac859ab1ae0c9503328fc99a6d0) used a solution of sending a no-op state update to avoid updating text for placeholders, when the Attributed strings are equal (though as of now this code is never reached, since we return earlier on AttributedString equality). I co-opted this mechanism, to avoid sending text updates if the text content and attributes of the AttributedString has not changed, disregarding any layout information. This is how the comparison worked at the time of the diff.

I also updated the fragment hashing function to include layout metrics, since it was added to be part of the equality check, and is itself hashable.

Changelog:
[Android][Fixed] - Fix `AttributedString` comparison logic for TextInput state updates

Reviewed By: sammy-SC

Differential Revision: D37902643

fbshipit-source-id: c0f8e3112feb19bd0ee62b37bdadeb237a9f725e
2022-07-18 18:20:22 -07:00
Vincent Riemer 966f800b7c Add key modifier properties to the PointerEvent interface
Summary:
Changelog: [iOS][Internal] - Add key modifier properties to the PointerEvent interface

This diff adds implementations of the `ctrlKey`, `shiftKey`, `altKey`, and `metaKey` properties on the PointerEvent interface for iOS.

Reviewed By: lunaleaps

Differential Revision: D37869377

fbshipit-source-id: b187bae93fbfc97b6ca1d8d9786ad85343484b3d
2022-07-18 14:12:25 -07:00
Samuel Susla bf54f1cae9 Back out "Fix default value for scrollView.contentInsetAdjustmentBehavior"
Summary:
changelog: [internal]

Original commit changeset: 290ae428a720

Original Phabricator Diff: D37881453 (https://github.com/facebook/react-native/commit/e98a835bfc7b01bccb1b7cdee94db248a0a90607)

in the original diff I made a mistake. The default value for RN is never, we override it in one of our subclasses.

Reviewed By: cortinico

Differential Revision: D37884715

fbshipit-source-id: 3c5b5ef5550120034e33ae9047292f38159dea3e
2022-07-15 09:54:35 -07:00
Samuel Susla e98a835bfc Fix default value for scrollView.contentInsetAdjustmentBehavior
Summary:
Changelog: [internal]

The dafault UIKit value of `UIScrollView.contentInsetAdjustmentBehavior` is automatic. Fabric had incorrect value never.

source: https://developer.apple.com/documentation/uikit/uiscrollview/2902261-contentinsetadjustmentbehavior?language=objc

This was causing bug for one navigation library that's migrating to Fabric: https://github.com/reactwg/react-native-new-architecture/discussions/43#discussioncomment-2885450

Reviewed By: javache

Differential Revision: D37881453

fbshipit-source-id: 290ae428a720f00ed61f2a3e5c2c9bbf54e1ac6e
2022-07-15 06:42:50 -07:00
Vincent Riemer 1eebbbc803 Implement offsetX/offsetY properties on the PointerEvent interface
Summary:
Changelog: [iOS][Internal] - Implement offsetX/offsetY properties on the PointerEvent interface

Simple diff implementing the offsetX/offsetY properties on PointerEvent — thankfully the touch events already kept track of these so it was mostly a matter of forwarding that data to the pointer events.

Reviewed By: lunaleaps

Differential Revision: D37830139

fbshipit-source-id: 77f33a99393350d32cbe449e6a009bdeb2a12d08
2022-07-14 15:07:18 -07:00
Michael Anthony Leon 753038cf34 Use RuntimeConfig for microtasks
Summary:
Changelog:
[general][change] - Use RuntimeConfig instead of VM Experiment Flag to set up the micro task queue.

Reviewed By: neildhar

Differential Revision: D37353947

fbshipit-source-id: 5c8f35c0a79d70cb0d234e881e55058cffb44ac8
2022-07-13 20:59:16 -07:00
Vincent Riemer 5665a77152 Implement screenX/screenY properties on the PointerEvent interface
Summary:
Changelog: [iOS][Internal] - Implement screenX/screenY properties on the PointerEvent interface

This diff implements the screenX/screenY properties which report the position of a pointer in the device's physical screen coordinates.

Reviewed By: lunaleaps

Differential Revision: D37794415

fbshipit-source-id: 6c39c3651812f99e66b93647579a2935598ef6f2
2022-07-13 18:09:22 -07:00
Pieter De Baets a42170e0f8 Replace SharedProps with Props::Shared
Summary:
Similarly to D37653146 (https://github.com/facebook/react-native/commit/13a0556aaa3976245ec065b0da68ef865bd03b70), standardize on one type to refer to this.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D37658743

fbshipit-source-id: 9834cfc4bbe4ca1a19f10c2c8718e3f74b88d9b6
2022-07-13 07:09:34 -07:00
Genki Kondo 5a47463e1f Prevent double application of opacity and backgroundColor on text
Summary:
The new props parsing path (introduced in https://www.internalfb.com/diff/D37051020 (https://github.com/facebook/react-native/commit/af1eae9ea313b016f111a25b5252b70aea99aedd)) resulted in opacity and backgroundColor being applied twice.

Specifically, the new prop setter path basically results in the ParagraphProps' textAttributes (via ConcreteComponentDescriptor.cloneProps, via ParagraphProps.setProp) to include opacity and backgroundColor, which is not desired as they are both applied again on view.

Similar issue faced in the past: https://www.internalfb.com/diff/D19764144 (https://github.com/facebook/react-native/commit/8fe6883fea5d92c8378a9642e8369c56f20872a7)

Changelog:
[Internal] - Prevent double application of opacity and backgroundColor on text

Reviewed By: ryancat

Differential Revision: D37801663

fbshipit-source-id: 4b5ebe5ff20b80833edb2141df883f33c69ab5d6
2022-07-12 20:01:52 -07:00
Vincent Riemer beca1a1c44 Add x/y & pageX/Y implementations to PointerEvent interface
Summary:
Changelog: [iOS][Internal] - add x/y & pageX/Y implementations to PointerEvent interface

This diff adds the x/y properties, which are defined as aliases of clientX/clientY respectively. In addition this diff adds pageX/pageY which, while not definted as aliases of clientX/clientY, are effectively aliases in React Native since the root view is not scrollable (client and page points only differ on the web when a user has scrolled the document element).

Reviewed By: lunaleaps

Differential Revision: D37766818

fbshipit-source-id: c7ad3750460b1913889c6d1a55b4c1edc6918f5b
2022-07-12 18:10:27 -07:00
Vincent Riemer 5989bca8ae Add twist property to PointerEvent interface
Summary:
Changelog: [iOS][internal] - Add twist property to PointerEvent interface

This adds the twist property to the PointerEvent implementation on iOS which similarly to tangentialPressure doesn't really apply so we default to a hard-coded value of 0.

Reviewed By: lunaleaps

Differential Revision: D37760511

fbshipit-source-id: f1fccfd6b5d07024cea83d86925a9bfc2e6cc8cf
2022-07-12 18:10:27 -07:00
Vincent Riemer 3cc335e615 Add tangentialPressure property to PointerEvent interface
Summary:
Changelog: [iOS][internal] - Add tangentialPressure property to PointerEvent interface

This diff adds the tangentialPressure property to the PointerEvent implementation on iOS. This one is pretty simple considering iOS doesn't have the concept of tangential pressure so we just hard code it to 0 as defined by the spec.

Reviewed By: lunaleaps

Differential Revision: D37759634

fbshipit-source-id: 7ca0e47267f5fde76ace2b96d05ea2e154cb4b8f
2022-07-12 18:10:27 -07:00
Pieter De Baets 13a0556aaa Remove deprecated ShadowNode type aliases
Summary:
Fix todo and inconsistency across codebase. It's better to have just one way to refer to these types.

Changelog: [Internal]

Reviewed By: philIip

Differential Revision: D37653146

fbshipit-source-id: e82f09caa6cd6eec5512b78f413708d9c04a7a83
2022-07-12 04:37:32 -07:00
Kevin Gozali 5e23ed699f Kill unused FBRotatablePhotoPlayer component
Summary:
This component is no longer relevant.

Changelog: [Internal]

Reviewed By: philIip

Differential Revision: D37733807

fbshipit-source-id: 267d69e5ffbb9bf9281f715b3e4e07c1f2f7ca9a
2022-07-08 19:06:39 -07:00
Vincent Riemer e89874c563 Add implementation of buttons property to PointerEvent interface
Summary:
Changelog: [iOS][Internal] - Add `buttons` implementation to the PointerEvent interface

This diff adds an implementation of the `buttons` property by leveraging `UIEvent`'s `buttonMask` property.

Reviewed By: lunaleaps

Differential Revision: D37430270

fbshipit-source-id: 69fd3aebcb403e665349a24283a04c0eb82ff3e2
2022-07-08 13:33:04 -07:00
Luna Wei 1753e7697d Fix: Ensure forms stacking context for events
Summary: Changelog: [Internal] - If any relevant view events (pointer, touch events, gesture responder, etc.) are declared on view, then the view must form stacking context. We need this change for pointer events specifically to determine whether we've entered/exited a view

Reviewed By: vincentriemer

Differential Revision: D37678352

fbshipit-source-id: 02641549ef608b1c9468ac693c7da629143212cb
2022-07-08 13:11:56 -07:00
Luna Wei d473881bd7 Fix: Clean up duplicate prop conversions
Summary: Changelog: [Internal] - Clean up duplicate properties from pointer event rename

Reviewed By: javache

Differential Revision: D37662527

fbshipit-source-id: f310d16103f12cdfe4e02546ea156553c9cd5443
2022-07-08 13:11:56 -07:00
Ron Edelstein 0d915aa942 Explicitly set language to JAVA where it is missing [xplat] (round 1)
Reviewed By: IanChilds

Differential Revision: D37594044

fbshipit-source-id: 0bbcaaed951a212651d3cc0fc3371751ced13852
2022-07-06 09:07:07 -07:00
Luna Wei 8be49e8746 PointerEvents: Remove '2' suffix
Summary: Changelog: [Internal] - We can now remove the '2' suffix as we had an internal implementation that was not truly aligned with W3C pointers but used the same name. We have aligned the internal types to match w3c so we can now remove the suffix that differentiates them.

Reviewed By: vincentriemer

Differential Revision: D37545813

fbshipit-source-id: 6f2336ae9e314066c340161113268c1f28621a71
2022-07-05 20:00:42 -07:00
Pieter De Baets 71e89213aa Make ReactCommon and tests compile for CXX platform
Summary:
If you don't list `Cxx` as a supported platform, we will use Android builds, even when using `buck run` for local execution.

Changelog: [Internal]

Reviewed By: derolf

Differential Revision: D37600464

fbshipit-source-id: 6ba8566cde4180524351c9d8c647ce1d4ac5279d
2022-07-05 05:56:35 -07:00
Joshua Gross b30ad9a77d Fix accessibilityRole prop parsing
Summary:
During the refactor in D36889794 (https://github.com/facebook/react-native/commit/782e004e4943194c40f2fb15527fe99a61f69b88) and later D37050215 (https://github.com/facebook/react-native/commit/47280de85e62f33f0b97bc1ed7c83bc6ca0dc7d4) this block of code was erroneously deleted; add it back, because if the props iterator setter is disabled, nothing sets this property anymore.

Changelog: [Internal]

Reviewed By: ryancat, nscoding

Differential Revision: D37539700

fbshipit-source-id: 0850180721e0549fed6089cb278402c56eb9d627
2022-06-29 19:07:17 -07:00
Joshua Gross 4f93f63f54 Fix non-deterministic crash in LayoutAnimations
Summary:
LA needs to ignore the new RemoveDeleteTree mutation coming from the differ.

Changelog: [Internal]

Reviewed By: jehartzog

Differential Revision: D37531217

fbshipit-source-id: c05b4106b6e955083e5e7e93619a13c4a2858404
2022-06-29 18:51:15 -07:00
Joshua Gross d1d11c7639 (Attempt to) Fix compilation of new props parsing for MSVC
Summary:
MSVC doesn't like Clang macros. Oops!

Constraints with this bit of code:

1. I'm trying to enforce constexpr in the most obvious, intuitive way possible. Macros are ugly but a "proper" solution would result in 10x as much code that is, totally subjectively, less readable to me.
2. This is evaluating at compile-time a hash of a string which is usually used in the `case` line of a switch statement.

For now I'm just hoping that MSVC will allow us to shadow `hash` which Xcode doesn't like. We might need to add a special pragma(s) for MSVC if it still doesn't like this.

Changelog: [Internal]

Reviewed By: lyahdav

Differential Revision: D37529949

fbshipit-source-id: 9aa605a9786bf5d194819ef8dade778875ae744e
2022-06-29 16:01:24 -07:00
Joshua Gross b6bbbf8efa RemoveDeleteTree mount instruction
Summary:
TL;DR: For applications using JS navigation, save 50-95% of CPU during mounting phase in N>2 navigations that replace ~most of screen.

During investigation of performance on the UI thread of React Native applications, I noticed that the /initial/ render of an screen for an application using JS navigation is /mostly/ consumed (on the UI thread) by tearing-down the previous View hierarchy. In one 185ms segment on the UI thread in production, 95% of the CPU time was Remove/Delete instructions and only 5% of CPU time was consumed by actually displaying the new hierarchy (this is specific to Android and also assumes that View Preallocation is being used, so post-commit work consists of Insert and UpdateLayout mutations primarily).

There are /some/ cases where the C++ differ knows that we are deleting an entire subtree and therefore we could communicate this to the mounting layer. All that matters is that these Views are removed from the View hierarchy immediately; and secondarily that their memory is cleaned up ASAP, but that doesn't need to happen immediately.

Some additional constraints and notes:

1) As noted in the comments, we cannot simply stop producing Remove and Delete instructions. We need to produce /both/ the new RemoveDeleteTree instruction, /and/ produce all the Remove/Delete instructions, primarily because LayoutAnimations relies heavily on these Remove/Delete instructions and certain things would break if we removed those instructions entirely. However, we can mark those Remove/Delete instructions as redundant, process them only in LayoutAnimations, and not send them to the Android mounting layer.
2) We want to make sure that View Recycling is not impacted. Since Android cannot take advantage of View Recycling until /after/ the second major render (preallocation of views will happen before any views are recycled), this doesn't impact View Recycling and we'll make sure Views are recycled whenever they are deleted.

Thus, we do two things:

1) Introduce a new RemoveDeleteTree operation that can delete an entire subtree recursively as part of one operation. This allows us to avoid serializing hundreds or thousands of instructions and prevents JNI traffic.
2) Besides removing the topmost View from the View hierarchy, and ensuring it's not drawn, the full teardown and recycling of the tree can happen /after/ the paint.

In some flows with JS navigation this saves us 95% of CPU during the mount phase. In the general case it is probably closer to 25-50% of CPU time that is saved and/or deferred.

Changelog: [Android][Changed] Significant perf optimization to Fabric Remove/Delete operations

Reviewed By: ryancat

Differential Revision: D37257864

fbshipit-source-id: a7d33fc74683939965cfb98be4db7890644110b2
2022-06-25 16:41:23 -07:00
Pieter De Baets a13cc68133 Fix runtimeScheduler definition of getCurrentPriorityLevel
Summary:
`getCurrentPriorityLevel` is a function which should return the priority level on invocation.

Changelog: [Internal]

Reviewed By: ryancat

Differential Revision: D37314727

fbshipit-source-id: fe385af02af49d1ca444beb881a4893f7f0712f0
2022-06-24 03:33:05 -07:00
Pieter De Baets a0d597dbca Throw error when accessing undefined property on runtimeScheduler
Summary:
This improves errors significantly, which is especially helpful as the runtime scheduler only implements a subset of the JS API.

Example error: `Error: Exception in HostObject::get for prop 'unstable_next': undefined property`

Changelog: [Internal]

Reviewed By: rickhanlonii

Differential Revision: D37313924

fbshipit-source-id: b53bc67b9cc36dee34dba86c07fdcf2353338c72
2022-06-23 03:37:56 -07:00
Vincent Riemer c25c4abe4a Add detail property to the PointerEvent object
Summary: Changelog: [iOS][Internal] Add detail property to the PointerEvent interface

Reviewed By: necolas

Differential Revision: D37117932

fbshipit-source-id: a5f1c6386d2521e22651453efeffe2005e4a8b6e
2022-06-21 14:05:44 -07:00
Vincent Riemer 27c004745b Add tiltX/tiltY properties to PointerEvent object
Summary: Changelog: [iOS][Internal] - Add tiltX/tiltY properties to the PointerEvent interface

Reviewed By: necolas

Differential Revision: D37117909

fbshipit-source-id: 277d1296b16bbf729dbc32f385634752fd145c8f
2022-06-21 14:05:44 -07:00
Luis Santana 68f3a42fc7 bump RTC-Folly to 2021.07.22 (#33841)
Summary:
Bumping RTC-Folly version used to address CVE-2022-24440.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General][Security] - Bump RTC-Folly to 2021-07-22

Pull Request resolved: https://github.com/facebook/react-native/pull/33841

Reviewed By: Andjeliko, philIip

Differential Revision: D36425598

Pulled By: cortinico

fbshipit-source-id: d38c5f020dbecf794b10f12ed2da30e1825071af
2022-06-21 12:36:43 -07:00
Kudo Chien f97c6a5b49 Fix broken use_frameworks from React-bridging (#34011)
Summary:
`use_frameworks!` is broken again in react-native 0.69 because React-bridging. in the `use_frameworks!` mode, header structures are flattened, so `#include <react/bridging/CallbackWrapper.h>` is not reachable to the header. to somehow workaround the issue without touch React-bridging imports, the pr do these things:

- use `header_mappings_dir` to keep `react/bridging` header structure
- because the header structure is not default framework header structure, explicitly `HEADER_SEARCH_PATHS` is necessary.
- forward declare `CallbackWrapper` and use it internally in ReactCommon. so that we don't need to add `HEADER_SEARCH_PATHS` for React-bridging to every pods depending on `ReactCommon/turbomodule/core`, e.g. React-RCTSettings.podspec.

## Changelog

[iOS] [Fixed] - Fix use_frameworks! for 0.69

Pull Request resolved: https://github.com/facebook/react-native/pull/34011

Test Plan:
```sh
$ npx react-native init RN069 --version next
# add `use_frameworks!` to ios/Podsfile
# comment out use_flipper!() in ios/Podfile
# patch node_modules/react-native with these changes
$ yarn ios
```

Reviewed By: cortinico, cipolleschi

Differential Revision: D37169699

Pulled By: dmitryrykun

fbshipit-source-id: 309c55f1c611a2fc3902a83e8af814daaf2af6a0
2022-06-21 02:56:42 -07:00
Ramanpreet Nara 168f020d5e Unbreak SurfaceRegistry users in bridge mode
Summary: Changelog: [Internal]

Reviewed By: nlutsenko

Differential Revision: D37250414

fbshipit-source-id: be3b98ba661c6f267d0d0dcac8d816584ef58c51
2022-06-17 15:06:31 -07:00
Ramanpreet Nara ea29ae1ceb SurfaceRegistryBinding: Display RedBox when RN$SurfaceRegistry isn't available
Summary:
SurfaceRegistryBinding::startSurface [checks whether global.RN$SurfaceRegistry binding is installed](https://www.internalfb.com/code/fbsource/[7040bef7d4fe43298c5f9dc1fef10f47ec396e79]/xplat/js/react-native-github/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp?lines=28-29). If not, control flow goes directly into the bridge path: [callFunctionOnModule](https://www.internalfb.com/code/fbsource/[7040bef7d4fe43298c5f9dc1fef10f47ec396e79]/xplat/js/react-native-github/ReactCommon/react/renderer/uimanager/SurfaceRegistryBinding.cpp?lines=40-46). callMethodOfModule [react_native_asserts, if the requested callable JS module isn't available](https://www.internalfb.com/code/fbsource/[7040bef7d4fe43298c5f9dc1fef10f47ec396e79]/xplat/js/react-native-github/ReactCommon/react/renderer/uimanager/bindingUtils.cpp?lines=29) on the batched bridge. This crashes the app.

We could make this error experience better:
1. We shouldn't crash the app.
2. We should fail fast, and produce an error message that is more descriptive of the actual cause of the error.
3. We can leverage the RedBox infra to display this error on the screen.

## Fixes
This diff modifies the gating inside SurfaceRegistryBinding. Now, in bridgeless mode, if global.RN$SurfaceRegistry isn't instaled, we'll display a RedBox instead, that shows what the error is.

Changelog: [Internal]

Reviewed By: sshic

Differential Revision: D37223640

fbshipit-source-id: 8fbf57f5d9cf359046dc94f0a5f7d66624caee4e
2022-06-17 04:34:59 -07:00
Ramanpreet Nara 4967e50989 Make LogBox render through SurfaceRegistry
Summary:
LogBox was using AppRegistry to render on to the screen. Switch LogBox over to using SurfaceRegistry instead.

Changelog: [Internal]

Reviewed By: sshic

Differential Revision: D37223641

fbshipit-source-id: 59001ad290c1e2c2f14828d38a96f48bd1ab39ca
2022-06-17 04:34:59 -07:00
Vincent Riemer 8cf57a5bdd Add width/height properties to PointerEvent object
Summary: Changelog: [iOS][Internal] Add width/height properties to the PointerEvent object

Reviewed By: kacieb

Differential Revision: D37116854

fbshipit-source-id: 686266d480bb2ee1d2b6696d80ad42865fa2111c
2022-06-16 13:05:54 -07:00
Vincent Riemer 033ffcc14b Remove unecessary PointerEvent attributes
Summary: Changelog: [iOS][Internal] Remove unecessary PointerEvent attributes

Reviewed By: kacieb

Differential Revision: D37115465

fbshipit-source-id: 079a297d1ce5b3d2c6766036a111c73bd75535f1
2022-06-16 13:05:54 -07:00
Joshua Gross af1eae9ea3 New Props parsing infrastructure for perf improvements: visitor pattern vs random-map-access pattern (BaseTextProps and derived props)
Summary:
See commentary at top of stack.

Changelog: [Added][Fabric] New API for efficient props construction

Reviewed By: javache

Differential Revision: D37051020

fbshipit-source-id: 643e433c0d0590cfcd17bc7a43d105bed6ff12ef
2022-06-15 23:37:34 -07:00
Joshua Gross 2ab585ac26 New Props parsing infrastructure for perf improvements: visitor pattern vs random-map-access pattern (ScrollViewProps)
Summary:
See commentary at top of stack.

Changelog: [Added][Fabric] New API for efficient props construction

Reviewed By: javache

Differential Revision: D37050961

fbshipit-source-id: 170a09c08d7406b6aac51d7e78cf295a72fdcf91
2022-06-15 23:37:34 -07:00
Joshua Gross 9f8c45deb7 New Props parsing infrastructure for perf improvements: visitor pattern vs random-map-access pattern (YogaLayoutableShadowNode / YGStyle)
Summary:
See commentary at top of stack.

Changelog: [Added][Fabric] New API for efficient props construction

Reviewed By: javache

Differential Revision: D37050376

fbshipit-source-id: 2bea35a6d604704cf430bd3b2914988227d1abf8
2022-06-15 23:37:34 -07:00
Joshua Gross 47280de85e New Props parsing infrastructure for perf improvements: visitor pattern vs random-map-access pattern (ViewProps, minus YogaLayoutableShadowNode)
Summary:
Perf numbers for this stack are given in terms of before-stack and after-stack, but the changes are split up for ease of review, and also to show that this migration CAN happen per-component and is 100% opt-in. Most existing C++ components do not /need/ to change at all.

# Problem Statement

During certain renders (select critical scenarios in specific products), UIManagerBinding::createNode time takes over 50% of JS thread CPU time. This could be higher or lower depending on the specific product and interaction, but overall createNode takes a lot of CPU time. The question is: can we improve this? What is the minimal overhead needed?

The vast, vast majority of time is taken up by prop parsing (specifically, converting JS values across the JSI into concrete values on the C++ props structs). Other methods like appendChild, etc, do not take up a significant amount of time; so we conclude that createNode is special, and the JSI itself, or calling into C++, is not the problem. Props parsing is the perf problem.

Can we improve it? (Spoiler: yes)

# How does props parsing work today?

Today, props parsing works as follows:

1. The ConcreteComponentDescriptor will construct a RawPropsParser (one per component /type/, per application: so one for View, one for Image, one for Text... etc)
2. Once per component type per application, ConcreteComponentDescriptor will call "prepare" on the RawPropsParser with an empty, default-constructed ConcreteProps struct. This ConcreteProps struct will cause RawProps.at(field) for every single field.
3. Based on the RawProps::at calls in part 2, RawPropsParser constructs a Map from props string names (width, height, position, etc) to a position within a "value index" array.
4. The above is what happens before any actual props are parsed; and the RawPropsParser is now ready to parse actual Props.
5. When props are actually being parsed from a JSI dictionary, we now have two phases:
  1. The RawPropsParser `preparse`s the RawProps, by iterating over the JSI map and filling in two additional data structures: a linear list of RawValues, and a mapping from the ValueIndex array (`keyIndexToValueIndex_`; see step 3) to a value's position in the values list (`value_` in RawPropsParser/RawProps);
  2. The ConcretePropT constructor is called, which is the same as in step 2/3, which calls `fieldValue = rawProps.at("fieldName")` repeatedly.
  3. For each `at` call, the RawProps will look up a prop name in the Map constructed in step 3, and either return an empty value, or map the key name to the `keyIndexToValueIndex_` array, which maps to a value in `values_`, which is then returned and further parsed.

So, a few things that become clear with the current architecture:

1. Complexity is a property of the number of /possible/ props that /can/ be parsed, not what is actually used in product code. This violates the "only pay for what you use" principal. If you have `<View opacity={0.5} />`, the ViewProps constructor will request ~170 properties, not 1!
2. There's a lot of pre-parsing which isn't free
3. The levels of indirection aren't free, and make cache misses more likely and pipelining is more challenging
4. The levels of indirection also require more memory - minor, but not free

# How can we improve it?

The goal is to improve props parsing with minimal or zero impact on backwards-compability. We should be able to migrate over components when it's clear there's a performance issue, without requiring everything gets migrated over at once. This both (1) helps us prove out the new architecture, (2) derisks the project, (3) gives us time, internally and externally, to perfect the APIs and gradually migrate everything over before deleting the old infrastructure code entirely.

Thus, the goal is to do something that introduces a zero-cost abstraction. This isn't entirely possible in practice, and in fact this method slightly regresses components that do not use the new architecture /at all/, while dramatically improving migrated components and causing the impact of the /old/ architecture to be minimal.

# Solution

1. We still keep the existing system in place entirely.
2. After Props are constructed (see ConcreteComponentDescriptor changes) we iterate over all the /values/ set from JS, and call PropsT::setProp. Incidentally, this allows us to easily reuse the same prop for multiple values for "free", which was expensive in the old system.
3. It's worth noting that this makes a Props struct "less immutable" than it was before, and essentially now we have a "builder pattern" for Props. (If we really wanted to, we could still require a single constructor for Props, and then actually use an intermediate PropsBuilder to accumulate values - but I don't think this overhead would be worth for the conceptual "immutability" win, and instead a "Construct/Set/Seal" model works fine, and we still have all the same guarantees of immutability after the parsing phase)

# Implementation Details
# How to properly construct a single Prop value

Minor detail: parsing a single prop is a 3-step process. We imagine two scenarios: (1) Creating a new ShadowNode/Props A from nothing/void, so the previous Props value is just the default constructor. (2) Cloning a ShadowNode A->B and therefore Props A must be copied to Props B before parsing.

We will denote this as a clone from A->B, where A may or may not be a previous node or a default-constructed Props node; and imagine in particular that we're setting the "opacity" value for PropsB.

We must first (1) copy a value over from the previous version of the Props struct, so B.opacity = A.opacity; (2) Determine if opacity has been set from JS. If so, and there is a value, B.opacity = parse(JSValue). (3) If JS has passed in a value for the prop, BUT the value is `null`, it means that JS is resetting or deleting the prop, so we must set it BACK to the default. In this case we set PropsB.opacity = DefaultConstructedProps.opacity.

We must take care in general to ensure that the correct behavior is achieved here, which should help to explain some of the code below.

## String comparisons vs hash comparisons

In the previous system, a RawPropsKey is three `const char*` strings, concatenated together repeatedly /at runtime/. In practice, the ONLY reason we have the prefix, name, suffix Key structure is for the templated prop parsing in ViewProps and YogaStyableProps - that's it. It's not used anywhere else. Further, the key {"margin", "Left", "Width"} is identical to the key {"marginLeftWidth", null, null} and we don't do anything fancy with matching prefixes before comparing the whole string, or similar. Before comparison, keys are concatenated into a single string and then we use `strcmp`. The performance of this isn't terrible, but it's nonzero overhead.

I think we can do better and it's sufficient to compare hashed string values; even better, we can construct most of these /at compile time/ using constexpr, and using `switch` statements guarantee no hash collisions within a single Props struct (it's possible there's a collision between Props.cpp and ViewProps.cpp, for example, since they're different switch statements). We may eventually want to be more robust against has collisions; I personally don't find the risk to be too great, hash collisions with these keys are exceedingly unlikely (or maybe I just like to live dangerously). Thus, at runtime, each setProp requires computing a single hash for the value coming from JS, and then int comparisons with a bunch of pre-compiled values.

If we want to be really paranoid, we could be robust to hash collisions by doing `switch COMPILED_HASH("opacity"): if (strcmp(strFromJs, "opacity") == 0)`. I'm happy to do this if there's enough concern.

## Macros

Yuck! I'm using lots of C preprocessor macros. In general I found this way, way easier in reducing code and (essentially) doing codegen for me vs templated code for the switch cases and hashing prop names at compile-time. Maybe there's a better way.

Changelog: [Added][Fabric] New API for efficient props construction

Reviewed By: javache

Differential Revision: D37050215

fbshipit-source-id: d2dcd351a93b9715cfeb5197eb0d6f9194ec6eb9
2022-06-15 23:37:34 -07:00