Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36164
Changelog:
[General][Fixed] - Invalid prop values no longer trigger assertion failures in Fabric
## Context
Fabric has historically been very strict about prop parsing. It originally `abort()`ed on any prop value that failed to parse according to the expected type; this was replaced with dev-only assertions in D27540903 (https://github.com/facebook/react-native/commit/cb37562f8355b624e271c5b74416c257b0e62a00). We've received feedback that C++ assertions (and other similar mechanisms in the legacy renderer) are still too aggressive and disruptive as a diagnostic for developers working on JS code.
We are changing React Native to behave more like a browser in this regard, reflecting a new principle that **bad style values are not runtime errors**. (See e.g. D43159284 (https://github.com/facebook/react-native/commit/d6e9891577c81503407adaa85db8f5bf97557db0).) The recommended way for developers to ensure they are passing correct style values is to use a typechecker (TypeScript or Flow) in conjunction with E2E tests and manual spot checks.
More broadly, values passed from JS product code should not be able to crash the app, which is why we're not strictly limiting this change to style props. From now on, if a JS developer can trigger an internal assertion in React Native simply by writing normal application code, that is a bug.
## This diff
This diff introduces a new macro called `react_native_expect` which serves as a drop-in replacement for `react_native_assert`, but logs (to glog / logcat / stdout) instead of asserting. This way we don't need to fully delete the existing call sites. This will be helpful if we decide that we want to repurpose these checks for a new, more visible diagnostic.
I'm *intentionally* opting for the simplest possible improvement here, which is to silence the assertions - not to print them to the JS console, not to convert them to LogBox warnings, etc. The hypothesis is that this is already strictly an improvement over the previous behaviour, will help us get to feature parity between renderers faster, and allow us to design improved diagnostics that are consistent and helpful.
## Next steps
1. There are still places where Fabric can hit an unguarded assertion in prop conversion code (e.g. unchecked casts from `RawValue` with no fallback code path). I will fix those in a separate diff.
2. Paper on iOS needs a similar treatment as it calls `RCTLogError` liberally during prop parsing (resulting in a native redbox experience that is nearly as bad as an outright crash). I will fix that in a separate diff.
3. I'll add some manual test cases to RNTester to cover these scenarios.
4. We will eventually need to take a clear stance on PropTypes, but since they provide reasonable, non-breaking diagnostics (recoverable JS LogBox + component stack) it is less urgent to do so.
Reviewed By: sammy-SC
Differential Revision: D43184380
fbshipit-source-id: 0c921efef297d935a2ae5acc57ff23171356014b
Summary:
changelog: [internal]
There was a bug in prop parsing where `accessibilityRole` and `accessibilityTraits` had wrong default value. Instead of falling back to sourceProps, they would set role to empty and trait to none. The fix is to fall back to sourceProps.
The issue surfaces with Animated, which clones props.
Reviewed By: cipolleschi
Differential Revision: D43190775
fbshipit-source-id: ec6b48c082c1dcc5b6e81dbff4f92b3a58a41da2
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36051
[Changelog][Internal]
This has been on my backlog for some time, submitting the diff to get it out of the way.
It makes the macro-based code in the "iterator-based property parsing" branch somewhat less horrible and less error prone (by removing duplication vs the default values in the class declaration).
Reviewed By: sammy-SC
Differential Revision: D42990595
fbshipit-source-id: e4b91160c6e09d3d1eab2ba70a58d390243bb335
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35953
DimensionValue is a reserved prop type that can be a number or string (such as '50%'). On Java, it will get converted to a YogaValue (converter added to this diff); on C++ it will get converted to a YGValue (converter already exists as it's used in Fabric).
Changelog:
[Internal][Added] - Add codegen support for DimensionValue for components
Reviewed By: cipolleschi
Differential Revision: D42650799
fbshipit-source-id: 1d2bc30bbd93837dedbbb4c74f814963c8140957
Summary:
This PR implements logical border-radius as requested on https://github.com/facebook/react-native/issues/34425. This implementation includes the addition of the following style properties
- `borderEndEndRadius`, equivalent to `borderBottomEndRadius`.
- `borderEndStartRadius`, equivalent to `borderBottomStartRadius`.
- `borderStartEndRadius`, equivalent to `borderTopEndRadius`.
- `borderStartStartRadius`, equivalent to `borderTopStartRadius`.
## Changelog
[GENERAL] [ADDED] - Add logical border-radius implementation
Pull Request resolved: https://github.com/facebook/react-native/pull/35572
Test Plan:
1. Open the RNTester app and navigate to the `RTLExample` page
2. Test the new style properties through the `Logical Border Radii Start/End` section
https://user-images.githubusercontent.com/11707729/206623732-6d542347-93f9-40da-be97-f7dcd5f66ca9.mov
Reviewed By: necolas
Differential Revision: D42002043
Pulled By: NickGerleman
fbshipit-source-id: a0aa9783c280398b437aeb7a00c6eb3f767657a5
Summary:
[Changelog][Internal]
The diff changes underlying storage types used by different enums that are used in ViewProps data structure, together with some eventual field rearranging to reduce padding overhead.
This **shaves off 128 bytes** from each `ViewProps` instance, which is **a ~10% improvement**.
Given that an average RN app may have thousands of shadow tree nodes, and correspondingly `ViewProps` instances in flight (e.g. Oculus Store has 2-3K of them nominally), the overall memory win is about **300K+** for this change only. Plus slightly better cache locality, which never a bad thing either.
Reviewed By: mdvacca
Differential Revision: D42372127
fbshipit-source-id: d3a832af2b2e89f50a5b8e04d24d0df92869ea4d
Summary:
This PR implements `inset` logical properties as requested on https://github.com/facebook/react-native/issues/34425. This implementation includes the addition of the following style properties
- `inset`, equivalent to `top`, `bottom`, `right` and `left`.
- `insetBlock`, equivalent to `top` and `bottom`.
- `insetBlockEnd`, equivalent to `bottom`.
- `insetBlockStart`, equivalent to `top`.
- `insetInline`, equivalent to `right` and `left`.
- `insetInlineEnd`, equivalent to `right` or `left`.
- `insetInlineStart`, equivalent to `right` or `left`.
## Changelog
[GENERAL] [ADDED] - Add Fabric implementation of inset logical properties
Pull Request resolved: https://github.com/facebook/react-native/pull/35692
Test Plan:
1. Open the RNTester app and navigate to the `View` page
2. Test the new style properties through the `Insets` section
<table>
<tr>
<td>Android</td>
<td>iOS</td>
</tr>
<tr>
<td><img src="https://user-images.githubusercontent.com/11707729/208821212-fbbac6ed-09a4-43f4-ba98-dfd2cbabf044.png" alt="1" width="360px" />
</td>
<td>
<img src="https://user-images.githubusercontent.com/11707729/208816997-ef044140-8824-4b1b-a77b-085f18ea9e0e.png" alt="2" width="360px" />
</td>
</tr>
</table>
Reviewed By: NickGerleman
Differential Revision: D42193661
Pulled By: ryancat
fbshipit-source-id: 3db8bcd2c4db0ef4886b9ec49a46424d57362620
Summary:
changelog: Introduce setNativeProps to Fabric
Add support for `setNativeProps` in Fabric for backwards compatibility. It is still recommended to move away from `setNativeProps` because the API will not work with future features.
We can make step [Migrating off setNativeProps](https://reactnative.dev/docs/new-architecture-library-intro#migrating-off-setnativeprops) in migration guide optional.
Reviewed By: yungsters, mdvacca
Differential Revision: D41521523
fbshipit-source-id: 4d9bbd6304b8c5ee24a36b33039ed33ae1fc21f8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35342
This is a native implementation of the JS shimmed layout-specific properties in https://github.com/facebook/react-native/pull/35316.
There is an experiment splitting the prop splitting codepath in Fabric, so this change effectively has two implementations depending on whether `enablePropIteratorSetter` is enabled.
None of these changes make sense to propagate Yoga. `inlineEnd`, etc are already mapped to `YGEdgeStart` and `YGEdgeEnd`, but RN's mapping used a different name. Then `blockStart`, `blockEnd`, map directly to existing edges in all cases since we don't support a writing mode.
On web, the last value in the style which computes the given dimension is given precedence. E.g. if "left" comes after "start" it will be chosen. Yoga stylesheets are unordered, and precedence for edges is given based on specificity (left > start > horizontal > all).
We give precedence to new renamings (e.g. start to inlineStart), but to preserve consistent behavior, we give precedence to specific edges before overwriting them with flow relative ones (e.g. marginTop has precedence over marginBlockStar).
Changelog:
[General][Added] - Add Fabric implementation of flow-relative padding and margin
Reviewed By: javache
Differential Revision: D41267765
fbshipit-source-id: 896e2ed71fe8bf83aef00b0a9d70fd20b2ce47a7
Summary:
a lot of files were depending on Geometry even though they didn't use Vector, this will help with future modularity
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D41127194
fbshipit-source-id: 024519c638a1f0df3fdbfbdd937eac84b9adee6e
Summary:
This PR adds React native binding for https://github.com/facebook/yoga/pull/1116
## Changelog
[General] [Added] - Flex gap yoga bindings
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
Pull Request resolved: https://github.com/facebook/react-native/pull/34974
Test Plan:
Run rn tester app and go to view example. You'll find a flex gap example. Example location - `packages/rn-tester/js/examples/View/ViewExample.js`
### Tested on
- [x] iOS Fabric
- [x] iOS non-fabric
- [x] Android Fabric
- [x] Android non-fabric
To test on non-fabric Android, I just switched these booleans. Let me know if there's anything else I might have missed.
<img width="674" alt="Screenshot 2022-10-14 at 3 30 48 AM" src="https://user-images.githubusercontent.com/23293248/195718971-7aee4e7e-dbf0-4452-9d47-7925919c61dc.png">
Reviewed By: mdvacca
Differential Revision: D40527474
Pulled By: NickGerleman
fbshipit-source-id: 81c2c97c76f58fad3bb40fb378aaf8b6ebd30d63
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34869
Changelog: [Internal]
This merges all instances of `enablePropIteratorSetter` into a single one.
Both `AccesibilityProps` and `BaseTextProps` had their own instances if it, which is now redundant.
Reviewed By: cipolleschi
Differential Revision: D40062555
fbshipit-source-id: b6ccf5a9538612dd731a6f9c4eaceeebcb6d95be
Summary:
A follow up to D38708718 (https://github.com/facebook/react-native/commit/403fea25f65a38f4b4d8e0edcf89741b29e62059) review, this factors feature flags for Fabric core code into a separate file, `CoreFeatures`.
Keeping them together is arguably better for maintenance and makes code easier to reason about.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D40007784
fbshipit-source-id: 1885d5d6200575c6015f063d8b05813b18b47ffb
Summary:
Previously, ViewPropsMapBuffer conversions were hardcoded deep in Android infrastructrue. I've generalized this into a different mechanism to allow any Props struct to support MapBuffer props.
There are still some things that need to be cleaned up and this should be treated as experimental. One thing we likely want to do is remove the hardcoded IDs (fine for codegen'd code; less so for handwritten) and use compile-time-hashed IDs instead with human-readable string names.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D38708719
fbshipit-source-id: 64603dee7f21828be31346c555d99862dab304ea
Summary:
Instead of having a special flag just for View MapBuffer props, we now use one flag to indicate that MapBuffer should be used for all props; each XShadowNode must set a special trait indicating if that ShadowNode supports MapBuffer props.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D38708718
fbshipit-source-id: b398ec62a0db9c0ff23c0007c5503cf2838c4173
Summary:
changelog: [internal]
Add more clang tidy rules to prevent common class of bugs.
Reviewed By: javache
Differential Revision: D39245194
fbshipit-source-id: 5521c5c4653d7005b96ebba494e810ba5075afbc
Summary:
Changelog: [iOS][Internal] - Add iOS implementation of the button property of the PointerEvent object
This implements the `button` property on the PointerEvent object by storing the button which caused the down event in the `ActiveTouch` and reporting that button through `pointerdown` and `pointerup` events and -1 on all others. This diff also includes a small fix to the `pressure` property which was introduced due to `button` being correctly implemented.
Reviewed By: yungsters
Differential Revision: D38632543
fbshipit-source-id: 9dbbb23a9251f2e661faf37fdf206b9f0b26bc5f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34403
This change mirrors D38457812 (https://github.com/facebook/react-native/commit/063c2b4668b279ccbca639f98f7a0a5c4d7c5690) which added -Wpedantic to ReactCommon targets, but for the Android build used by OSS. This should ensure contributors see the same warnings locally as the internal build would produce.
Changelog:
[Android][Changed] - Enable -Wpedantic in OSS Android Targets
Reviewed By: cortinico
Differential Revision: D38632454
fbshipit-source-id: 19a036ee3f902eb9d47c568aef448af9d8562358
Summary:
Folly's molly target combines a number of targets that are supposed to be useable on mobile. Since we're trying to move away from folly, we should instead list explicitly which parts of folly we're using so we can remove them over time, and track which targets no longer have any folly dependencies.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D38352060
fbshipit-source-id: 09d0d84793692f97f4d49390c99c38b23441df54
Summary:
React Native is compiled downstream with MSVC, meaning the introduction of code depending on language extensions specific to gcc/clang may cause breakage.
We can enable `-Wpedantic` to flag any behavior not officially supported by the specified C++ standard. This will includes rules beyond what MSVC has trouble with, but seems to not have too many "noisy warnings".
This change enables -Wpedantic in BUCK targets within ReactCommon.
This makes the OSS C++ build for Android/iOS slightly more permissive than the internal build, A followup is to add the changes to OSS build logic as well, to avoid contributors seeing more errors upon internal submission. (checking with cortinico on how to do this for Android).
react-native-windows uses a higher warning level than `-Wall`, which is an additional cause of compilation failures, but is not addressed as part of this change.
Changelog:
[Internal][Changed] - Enable -Wpedantic for targets inside ReactCommon
Reviewed By: rshest
Differential Revision: D38457812
fbshipit-source-id: 014da1ac0b7ad8f78154e6e447ed58def6bd0d47
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
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
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
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
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
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
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
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
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
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
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
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
Moti Zilberman
Samuel Susla
Ruslan Shestopalyuk
gabrieldonadel
Genki Kondo
Nishan
Phillip Pan
Nick Gerleman
David Vacca
Jonathan Keljo
Samuel Susla
Luna Wei
Vincent Riemer
Nick Gerleman
Pieter De Baets
Ruslan Shestopalyuk
Chiara Mooney
Eric Edouard
Joshua Gross
