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:
changelog: [internal]
For some reason, using `TextLayoutManager::Shared` in `TextInputShadowNode` trips up clang tidy linter. We have a plan to move away from `*::Shared` anyway, so let's remove it from `TextInputShadowNode` now.
Why do we want to move away from `*::Shared`?
Using `TextLayoutManager::Shared` is confusing for people unfamiliar with Fabric's codebase. It expresses a concept of immutability but uses term `shared`. Term shared is already used in C++ `std::shared_ptr`.
Reviewed By: fkgozali
Differential Revision: D33186422
fbshipit-source-id: 10ee588735997f5fedc372a1d1e3d9cd9684178a
Summary:
Just clarifying the purpose of this library.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D33245053
fbshipit-source-id: b4325415774ca33b34b5ba53d222a19f88e2175f
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: Changelog: [Internal] - Update the source of the changes in generated files, no longer bump-oss-version but set-rn-version
Reviewed By: sota000
Differential Revision: D33110408
fbshipit-source-id: 8cd5004f5d40dde82fe4d6271d5b8598cd27ca31
Summary:
changelog: [internal]
Only execute tasks that are expired when in synchronous mode.
Reviewed By: philIip
Differential Revision: D33062746
fbshipit-source-id: 1825cb572202d1f5dc18eb2b481dd3c20e8e91ba
Summary:
Allows `CxxModule` objects to set their `instance_` member before `getMethods()` is invoked, allowing for the `Method` callbacks to capture a weak reference to `getInstance()` if needed.
## 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] [Fixed] - Set CxxModules' Instance before retrieving their Method vector.
Pull Request resolved: https://github.com/facebook/react-native/pull/32719
Test Plan: Ensure this statement swap does not break any scenarios with existing CI validation.
Reviewed By: JoshuaGross
Differential Revision: D32955616
Pulled By: genkikondo
fbshipit-source-id: fd7a23ca2c12c01882ff1600f5aef86b1fe4a984
Summary:
changelog: [internal]
Just moving code that doesn't belong to UIManagerBinding out of the class.
Reviewed By: philIip
Differential Revision: D33060412
fbshipit-source-id: 2d54929072cef14fd1fa6b70bde382ae21ecff45
Summary:
Fabric uses a cache where it stores the result of the text measurement in C++ (to avoid unnecessary text measurement that are very costly). This cache has a "max size" of 256 and this size is not enough to store all the texts we have in the screen
In my tests, the amount of texts being measured are ~290 and after scrolling many times they increase to 611.
This diff increases the size of the TextMeasure to 1024 for users in the experiment. As a result this improves performance of HoverEvents by +5x times (see test plan)
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D33112788
fbshipit-source-id: e15feecf0f54da62b252892d37a64fb4ead29e22
Summary:
Applies suggestions from default preset that make sense in our codebase
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D33010231
fbshipit-source-id: 6bc9edf01fd9bd9938d211e3494dd1a127b24eaa
Summary:
Follow up to https://github.com/facebook/react-native/issues/32683 to also link hermes-inspector statically.
## Changelog
[Android] [Fix] - Static link for hermes-inspector
Pull Request resolved: https://github.com/facebook/react-native/pull/32694
Test Plan: Tested a clean build and made sure hermes-inspector.so doesn't exist anymore.
Reviewed By: cortinico
Differential Revision: D32791208
Pulled By: ShikaSD
fbshipit-source-id: 6076b263469b9e2c6176d488d13292d4f1731dcc
Summary:
changelog: [internal]
Provide `UIManager` to `UIManagerBinding` in constructor to make the API safer.
Reviewed By: philIip
Differential Revision: D32668892
fbshipit-source-id: a15cd295196a60c3f46997e59c05c4f90503e18d
Summary:
The IDE warning suggests that passing folly::dynamic by value will create a copy on each call.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D32978154
fbshipit-source-id: a47a60c332a9d299eb2110d3537dfab0bc2398b6
Summary:
changelog: [internal]
Introduce a way to execute `onKeyPress` synchronously. This feature is experimental and will be changed in the future. It is not decided if marking native events as "sync" is going to be path forward with synchronous access.
NOTE: This is experimental API.
Reviewed By: ShikaSD
Differential Revision: D32882092
fbshipit-source-id: 68c66a9bb7c97758219e085c88a77f3c475c1eb3
Summary:
changelog: [internal]
Event `onChangeText` does not exist in TextInput. Let's remove this method to avoid confusion.
Reviewed By: philIip
Differential Revision: D32882056
fbshipit-source-id: 37eb260b84dd7d6cce412ce1bc39c0cbf9cab112
Summary:
changelog: [internal]
Exposes a new flag on RuntimeScheduler: `unstable_getIsSynchronous`. Flag indicates if the current code is run synchronously and therefore commit phase should be synchronous.
Unit tests will be added later, to keep this diff short. This code path is not executed yet.
Reviewed By: mdvacca, ShikaSD
Differential Revision: D32677814
fbshipit-source-id: e01d4fff7e716d627ff99fe104965851138c3aef