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:
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:
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:
Add unit tests for Layout Animations.
This first batch generates a random mutation, then animates it to completion.
I found one issue with UPDATE+REMOVE+INSERT animation consistency. That shouldn't cause any crashes in production, but is a chance to improve consistency of mutations overall - and could in theory point to memory corruption, though it's somewhat unlikely.
I ran with randomized seeds, found issues, fixed them, re-ran to ensure issues were fixed, rinsed and repeated. At the end I was able to run dozens of times (with random seeds) and found nothing.
The next step is to repeatedly generate mutations that conflict with ongoing animations.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28343750
fbshipit-source-id: c1c60d89a31be3ac05d57482f0af3c482b866abe
Summary:
Change the `TransactionTelemetryTest` to use a mock clock or change the tests to only test if at least x amount of time has passed.
Using a mock clock is the only way to make the test deterministic and be able to assert on the sub-results of the captured phases.
Changelog: [Internal] Change to the `TelemetryTest`. Neither changes the runtime behavior nor the API.
Reviewed By: sammy-SC
Differential Revision: D27618448
fbshipit-source-id: 0cbf51b050aabb75341112ea4a43bea0115082f9
Summary:
The `TransactionalTelemetryTest`s are flaky because they use a real clock and assert on how much time has passed (with a threshold, but that's no good either).
Using the real clock in the test is the cause for the test to be flaky because it depends on the assumption that it's the sole process running, never risking to be put in the process-queue of the OS. However, that's not the case and is why the test sporadically fails if the OS decided to schedule other threads/process in the middle of the test.
This diff allows parametrising the `TransactionTelemetry` class with the clock implementation so that tests can use a Mock Clock if desired (separate diff).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D27618516
fbshipit-source-id: 5a08e115b388398ca2b05b9d5ae0fd281dfe3b04
Summary:
The current test failures don't include the values passed to `EXPECT` which makes it difficult to understand if the test ended earlier or later then expected.
```
Failure: Value of: (commitDuration >= 1000 - threshold) && (commitDuration <= 1000 + threshold)
Actual: false
Expected: true
```
This diff uses the gtest `EXPECT_NEAR` to get exception messages including the delta:
```
Failure: The difference between telemetryDurationToMilliseconds(telemetry.getTextMeasureTime()) and 600 is 153, which exceeds threshold, where
telemetryDurationToMilliseconds(telemetry.getTextMeasureTime()) evaluates to 753,
600 evaluates to 600, and
threshold evaluates to 70.
```
This doesn't change the test's flakiness because of how sleep is implemented.
Changelog: [Internal] Test only change
Reviewed By: sammy-SC
Differential Revision: D27595206
fbshipit-source-id: f31bdd92ecc7271c9491dda18639ea08820f5730
Summary:
Changelog: [Internal] enable support for C++ 17.
C++ 17 in React Native targets.
Short and comprehensive list of C++ features:
https://github.com/AnthonyCalandra/modern-cpp-features#c17-language-features
Reviewed By: JoshuaGross
Differential Revision: D27431145
fbshipit-source-id: e8da6fe9d70e9b7343a8caec21cdbeb043478575
Summary:
We need to do this to break a dependency cycle that would happen if we try to have `view` depend on `mounting` just to add some telemetry to `view`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26827446
fbshipit-source-id: 4c415ebf5be3a02c18c80ea8a4a77068cae0f0fe
Andres Suarez
Samuel Susla
Kevin Gozali
Nicola Corti
Joshua Gross
Micha Reiser