Summary:
While it would be better to be able to do all of the ownership metadata at the Buck macro level, that proved to be more work than expected.
This diff adds the corresponding pfh label to all targets in `xplat/js/react-native-github` that have a Supermodule label. Once the migration is complete the Supermodules labels will be able to be removed.
Reviewed By: cortinico
Differential Revision: D35221544
fbshipit-source-id: d87d5e266dfb5e6ee087251dc34dff5db299bbaf
Summary:
Adding more context for event dispatching process in systrace.
Changelog: [Internal]
Reviewed By: philIip
Differential Revision: D35208257
fbshipit-source-id: 4a70e15a0074d4a53a895066e6fa1e60a6ebda0d
Summary:
Changes in https://github.com/facebook/react-native/compare/7cece3423...189c2c895 broke build for Windows because of a conversion from size_t to int. Adds a static cast to int to fix the error and restore windows build
Error Message
```
##[error]node_modules\react-native\ReactCommon\react\renderer\core\RawPropsParser.cpp(100,42): Error C2220: the following warning is treated as an error
3>D:\a\_work\1\s\node_modules\react-native\ReactCommon\react\renderer\core\RawPropsParser.cpp(100,42): error C2220: the following warning is treated as an error [D:\a\_work\1\s\vnext\Microsoft.ReactNative\Microsoft.ReactNative.vcxproj]
##[warning]node_modules\react-native\ReactCommon\react\renderer\core\RawPropsParser.cpp(100,42): **Warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data**
3>D:\a\_work\1\s\node_modules\react-native\ReactCommon\react\renderer\core\RawPropsParser.cpp(100,42): warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data [D:\a\_work\1\s\vnext\Microsoft.ReactNative\Microsoft.ReactNative.vcxproj]
```
## Changelog
[General] [Fixed] - Restore Windows build with RawPropsParser.cpp
Pull Request resolved: https://github.com/facebook/react-native/pull/33432
Test Plan: Tested locally and changes pass in the react-native-windows pipeline, change is being merged into the main branch of react-native-windows.
Reviewed By: philIip
Differential Revision: D34907928
Pulled By: javache
fbshipit-source-id: 8b76cbef0b637f2d607a8aefd2998322c3245713
Summary:
This Diff moves from specifying a list of files to use file(GLOB) with
CONFIGURE_DEPENDS on several CMakefiles.
I've updates those where we use globbing also inside buck.
Changelog:
[Internal] [Changed] - Setup Globbing with CONFIGURE_DEPENDS inside CMake files
Reviewed By: ShikaSD
Differential Revision: D34826311
fbshipit-source-id: 8fc654626c897cdc4cdd79c699ce19f1e5e1212f
Summary:
Rearranges folly_futures configuration into a static library only required for `hermes-inspector` + `folly_runtime` which merges `folly_json` and mutex-related implementations `folly_futures` was used for. As `hermes-executor-debug` is removed by `vmCleanup` configurations later, it allows to shave additional 300KB from the release APK size.
Changelog: [Internal] - Rearrange folly build to reduce APK size
Reviewed By: cortinico
Differential Revision: D34342514
fbshipit-source-id: b646680343e6b9a7674019506b87b96f6007caf2
Summary:
This is the first round of CMake files to support the React Native build on Android.
They're supposed to eventually replace the various Android.mk files we have around in the codebase.
So far we're not actively using them. This is the first step towards migrating our
setup to use CMake
Changelog:
[Internal] [Changed] - First Round of CMake files for React Android
Reviewed By: ShikaSD
Differential Revision: D34762524
fbshipit-source-id: 6671e203a2c83b8874cefe796aa55aa987902a3b
Summary:
Changelog: [internal]
To avoid unnecessary string copy in event pipeline, use move semantics.
Event pipeline has ownership of event type. Passing it by reference ends up in a copy when `RawEvent` object is constructed. To avoid this, pass string by value through each layer and use move semantics to avoid extra copies.
Reviewed By: javache
Differential Revision: D34392608
fbshipit-source-id: c11d221be345665e165d9edbc360ba5a057e3890
Summary:
`size_` will always match `keys_.size()` so we don't have to keep track of it.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D34391445
fbshipit-source-id: f6c9316c989137425abfb7b3d72b571d08240f34
Summary:
With the `MapBuffer`-based props calculated from C++ props, there's no need to keep `rawProps` around for Android views.
This change makes sure that the `rawProps` field is only initialized under the feature flag that is responsible for enabling `MapBuffer` for prop diffing, potentially decreasing memory footprint and speeding up node initialization as JS props don't have to be converted to `folly::dynamic` anymore.
For layout animations, props rely on C++ values, so there's no need to update `rawProps` values either.
Changelog: [Internal][Android] - Do not init `rawProps` when mapbuffer serialization is used for ViewProps.
Reviewed By: mdvacca
Differential Revision: D33793044
fbshipit-source-id: 35873b10d3ca8b152b25344ef2c27aff9641846f
Summary:
Small performance improvement since we don't need to copy the char * into a string just for the sake of comparison.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D34351387
fbshipit-source-id: 10163164f6e95ab0737e8f865c37d8f0c3500662
Summary:
changelog: [internal]
You can read more about this rule on https://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html
# Isn't it wasteful to copy? Isn't reference more efficient?
This rule of thumb is no longer true since C++11 with move semantics. Let's look at some examples.
# Option one
```
class TextHolder
{
public:
TextBox(std::string const &text) : text_(text) {}
private:
std::string text_;
};
```
By using reference here, we prevent the caller from using rvalue to and avoiding copy. Regardless of what the caller passes in, copy always happens.
# Option two
```
class TextHolder
{
public:
TextBox(std::string const &text) : text_(text) {}
TextBox(std::string &&text) : text_(std::move(text)) {}
private:
std::string text_;
};
```
Here, we provide two constructors, one for const reference and one for rvalue reference. This gives the caller option to avoid copy. But now we have two constructors, which is not ideal.
# Option three (what we do in this diff)
```
class TextHolder
{
public:
TextBox(std::string text) : text_(std::move(text)) {}
private:
std::string text_;
};
```
Here, the caller has option to avoid copy and we only have single constructor.
Reviewed By: fkgozali, JoshuaGross
Differential Revision: D33276841
fbshipit-source-id: 619d5123d2e28937b22874650366629f24f20a63
Summary:
Renaming the `better` utilities to `butter`:
- to prevent claims that this library is superior to others - it really depends on use cases
- to indicate ease of use throughout the codebase, easily spread like butter
Changelog: [C++][Changed] Renaming C++ better util to butter, used by Fabric internals
Reviewed By: JoshuaGross
Differential Revision: D33242764
fbshipit-source-id: 26dc95d9597c61ce8e66708e44ed545e0fc5cff5
Summary:
changelog: [internal]
In an effort to make our codebase more approachable, I'm enabling more clang-tidy rules.
Read more about modernize-avoid-c-arrays in https://clang.llvm.org/extra/clang-tidy/checks/modernize-avoid-c-arrays.html
Reviewed By: ShikaSD
Differential Revision: D33187162
fbshipit-source-id: c6b3888f67d095274bb492a01132985ae506c0d5
Summary:
changelog: [internal]
I will be enabling more clang tidy rules in Fabric to make it easier for new contributors and standardise the codebase. \
You can read more about the rule in https://clang.llvm.org/extra/clang-tidy/checks/modernize-avoid-bind.html
Reviewed By: ShikaSD
Differential Revision: D33162192
fbshipit-source-id: b4bb332f3134c42c49559a8baf10aeb7a7fdd87f
Summary:
We have `LOCAL_SHARED_LIBRARIES` that are getting longer and are
making reviewing them on Diffs quite hard.
Having all the list of the dependency on a single line is suboptimal
and it makes hard to find duplicated entries.
I've updated the longest `LOCAL_SHARED_LIBRARIES` to be multilines and
I've sorted the entries here.
Changelog:
[Internal] [Changed] - LOCAL_SHARED_LIBRARIES
Reviewed By: ShikaSD
Differential Revision: D32695127
fbshipit-source-id: f5b381c501ddff083ef9f4baaca6c4c8c9523368
Summary:
These dynamic_casts aren't really giving us much (they have never fired once in dev! and don't run in prod anyway). They also prevent us from disabling RTTI. So, let's get rid of them.
Changelog: [Internal]
Reviewed By: philIip
Differential Revision: D31634895
fbshipit-source-id: 4a9b259837127feb324f64fa3e9e23eb1cc481a6
Summary:
Nearly all of these are identical and these compiler_flags are now centralized in rn_defs.bzl. This should have NO CHANGE on build configuration, the flags have just moved for now.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D31631766
fbshipit-source-id: be40ebeb70ae52b7ded07ca08c4a29f10a0ed925
Summary:
For iOS, event category deduction is done from the C++ code, but the touch events are handled on Java layer in Android. This change exposes the category parameter through the `EventEmitterWrapper` called from Java, allowing to define category for events in the future.
Changelog:
[Internal] - Expose event category through JNI
Reviewed By: mdvacca
Differential Revision: D31205587
fbshipit-source-id: f2373ce18464b01ac08eb87df8f421b33d100be2
Summary:
changelog: [internal]
LayoutAnimations only animates changes inside View and Paragraph nodes. This diff extends it to any node that's ViewKind.
Reviewed By: JoshuaGross
Differential Revision: D30603138
fbshipit-source-id: 63ca1e5df420149c4ba66151e97fea419fdfe631
Summary:
changelog: [internal]
Use surfaceId type instead of raw int type in PropsParserContext
Reviewed By: RSNara
Differential Revision: D30667540
fbshipit-source-id: 2bf890b892851e785318e3566692ac72ec782b91
Summary:
Ship libjsi as a standalone dynamic library. This prevents problems
with exception handling caused by duplicate typeinfo across multiple
shared libs, and reduces bundle size by removing duplicate copies of
JSI.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D30599215
fbshipit-source-id: abad1398342a5328daa825f3f684e0067cad7a96
Summary:
LLD, our new iOS linker, is an ongoing effort to migrate our old outdated ld64 linker. As part of our effort to rollout LLD to all apps, we are making sure LLD reaches parity with ld64.
Due to Identical Code Folding (ICF), LLD and ld64 handles strings differently. LLD treats each string as a separate object in memory even if the values of the strings are the same. ld64 happens to aggregate these values across files. This behavior creates a subtle difference on our codebase when we start comparing by value or by reference.
`char * ` fields from `RawPropsKey.h` are using `==` which compares by its address. Here, we cast the buffer to a string to make the comparison, while avoiding the cast if one happens to be null.
Changelog: [Internal]
Reviewed By: int3, JoshuaGross
Differential Revision: D30444176
fbshipit-source-id: 74216926803adbece05206ddd8478cc3c8e6812e
Summary:
This diff is part of a bigger effort to remove the RTTI flags.
To do so we need to remove occurrences of `dynamic_cast` and other functions that rely on runtime
type informations.
Changelog:
[Internal][Changed] - Removed extra asserts relying on dynamic_cast
Reviewed By: JoshuaGross
Differential Revision: D30483554
fbshipit-source-id: 92b31281841a92c7b43e918938248431265dd654
Summary:
Fabric core uses a lot of traits - I am reserving a few more for core usage, and also exposing a few "unreserved" traits.
It is recommended that all custom components that do use traits rely on these constants instead of hard-coding any trait values. That way, in the unlikely event that these values change in the future, it will not break components.
Changelog: [Internal]
Reviewed By: cortinico, RSNara
Differential Revision: D30401743
fbshipit-source-id: fb2e8f5cf33c94e31a0c25a89055acfc4eccf066
Summary:
See previous diffs for context. This updates all of the relevant props structs.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D29855426
fbshipit-source-id: 30177c3380ef82ecf8f2a4321f128cfbe8a576e0
Summary:
Adding runtime type information adds greatly to the binary size, so react-native-windows builds without it. But some parts of the fabric code currently uses dynamic_cast, which means to use fabric we have to build with RTTI turned on. This PR removes the usages of dynamic_cast that are hit in release builds, which should allow react-native-windows to turn off RTTI in release builds.
Required for: https://github.com/microsoft/react-native-windows/issues/7981
One thing to note, the comment in ShadowNodeTraits indicates that core was reserving the first 16 bits. I'm adding two more. Is that ok? Should core be reserving more for future use?
## 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
-->
[Internal] [Fixed] - Remove uses of dynamic_cast in release builds
Pull Request resolved: https://github.com/facebook/react-native/pull/31694
Test Plan:
Verified that I can build react-native-windows with Fabric in release, without RTTI.
Boot / clicked around in RNW RNTester
Reviewed By: sammy-SC
Differential Revision: D29040383
Pulled By: JoshuaGross
fbshipit-source-id: e49286e59c4ba54faf0b4de5e244dfa1f7c3f193
Summary:
Changelog: [internal]
There is a possibility of race between updating scrollview's state and virtualised list asking for layout of individual cells.
To make sure the race doesn't happen, state must be updated before dispatching onScroll event.
Android has implemented a different mechanism to tackle this issue in D28558380 (https://github.com/facebook/react-native/commit/b161241db2ef74d2e4bff36d4972f5f0312dcc44).
Reviewed By: JoshuaGross
Differential Revision: D28642737
fbshipit-source-id: 33874beac69fc5a66eeb7f459fd89cd0b00dafcf
Summary:
Changelog: [internal]
This is a mechanism that will guess event's React priority based on other events ongoing on the platform.
If an event happens within span of ContinuousStart -> ContinuousEnd and its category is unspecified, we deduce it's React priority to be default. All other events are discrete.
Special case: `onScroll`, which is always treated as "Default".
Reviewed By: JoshuaGross
Differential Revision: D28485060
fbshipit-source-id: d2eae63dbcf03271dfed97128a1590dd165a3ce2
Summary:
Changelog: [internal]
Pulling out event queue processing to its separate class. We will be adding more logic there so it makes sense to separate it for simplicity and testability purposes.
Alternative names that came to mind: `EventsProcessor`, `EventsDispatcher`.
Reviewed By: JoshuaGross
Differential Revision: D28572283
fbshipit-source-id: 1cb2459f616b9995f66da80d50c401e68002da7f
Chris Olszewski
Xin Chen
Tatiana Kapos
Andrei Shikov
Nicola Corti
Samuel Susla
Pieter De Baets
Andres Suarez
Kevin Gozali
CodemodService FBSourceClangFormatLinterBot
Joshua Gross
Neil Dhar
Vincent Lee
David Vacca
Ikko Ashimine
Andrew Coates