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:
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]
Only execute tasks that are expired when in synchronous mode.
Reviewed By: philIip
Differential Revision: D33062746
fbshipit-source-id: 1825cb572202d1f5dc18eb2b481dd3c20e8e91ba
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:
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
Summary:
This class is not used and can be safely deleted
Changelog: [Internal] Delete unused Android event emitter
Reviewed By: mdvacca
Differential Revision: D32916706
fbshipit-source-id: 6dceb6b6ed9d201d96454bf0d646853c5c893d59
Summary:
changelog: [internal]
Background executor has been shipped on both platforms for a long time.
I've kept the flag around because I wanted to run tests and compare Concurrent Mode vs Background Executor. The intention was to see if we can get rid of Background Executor to simplify the threading model.
Since then, React team has moved away from Concurrent Mode towards more gradual rollout of concurrent rendering and it no longer makes sense to do this comparison. Right now, we don't have a concern with concurrent rendering and Background Executor. If we ever want to run the an experiment, this gating will need to be added again.
Reviewed By: javache
Differential Revision: D32674798
fbshipit-source-id: a1e51c9c5b8e48efa4cb0f25379d58e7eb80ccd9
Summary:
changelog: [internal]
UIManager::animationTick is thread safe. Let's make it obvious by marking the method const.
Reviewed By: javache
Differential Revision: D32669102
fbshipit-source-id: 49e35d0f0a5c5d1b03baa1cbf9cdece082909e85
Summary:
If `value` is of type `float`, it will still fall through to the `react_native_assert` below, exploding. The `map` type is handled correctly.
Changelog: [Internal][Fixed] Fix fromRawValue(EdgeInsets) from single float case falling through to float array and exploding
Reviewed By: javache
Differential Revision: D32648848
fbshipit-source-id: e70cddd291a8f52d6ee3de5fef11b0bb7aee92cd
Summary:
Removes extra .so files by merging built-in components into libfabricjni.so
These components shouldn't be referenced in outside modules, so merging them is trivial atm.
Changelog:
[Internal][Android] - Compile native components into static libraries
Reviewed By: cortinico
Differential Revision: D32677572
fbshipit-source-id: fc1a6c5a2832ee49e438c30856562f85677514ea
Summary:
Changelog: [internal]
Nothing was using `stop` parameter, let's get rid of it.
Reviewed By: philIip
Differential Revision: D32669018
fbshipit-source-id: dc2d52048a2f7dd3785dd959270087001c778962
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
Samuel Susla
Kevin Gozali
David Vacca
Andrei Shikov
Felipe Perez
Nicola Corti
Nawbc