Summary:
This fixes an error where folly fails to build on Xcode 12.5, by bumping the various folly deps in RN to builds with a fix.
Next step is to commit this to 0.64 release branch
allow-large-files
Changelog: [iOS] Fix builds on Xcode 12.5
Reviewed By: fkgozali
Differential Revision: D28071808
fbshipit-source-id: 236b66bf8294db0c76ff25b11632c1bf89525921
Summary:
Changelog: [internal]
state infra uses rvalue references until this point. I assume the original author intended to rvalue reference even here.
This way, we avoid unnecessary copy.
Reviewed By: JoshuaGross
Differential Revision: D28057570
fbshipit-source-id: 19af480234d44acffcdbb22606607279e25c8aed
Summary:
Refactor a code block that is duplicated 2x. Logic stays the same besides renaming, and a ternary operator to decide between getting the children from "old" or "new" tree.
Tests can help us refactor knowing that the logic is still correct.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28018994
fbshipit-source-id: d34a033444e67091e44ff6a747fd39846c165238
Summary:
There's a case here where we do a loop, with a map loopup, and nested map lookup inside of that. It's not particularly efficient and was done because we have multiple distinct pointers to distinct ShadowViews that are backed by the same ShadowNode. Now due to previous, recent refactoring, we can simplify this case a lot.
The code WAS correct before, just confusing and not particularly efficient. Tests can prove that this is still correct.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28018996
fbshipit-source-id: a7c8148802650c88888960c9c099954e0f8bc357
Summary:
This is no longer true because of the "scope" mechanism.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28018995
fbshipit-source-id: 91470234bb15f7feeb92b41613b0bbdbe42ccb27
Summary:
I am deduping a duplicated block, and adding comments to explain when we create INSERT/REMOVE mutations immediately and when we defer creation.
Theoretically the ordering of mutations will be more consistent now, which ~shouldn't matter, but is probably a decent property to have. In particular, before, in some cases
both of these orderings were possible in various scenarios:
```
INSERT X -> Y
INSERT Y -> Z
```
and
```
INSERT Y -> Z
INSERT X -> Y
```
Both of those are fine/correct/won't cause issues on any known platforms. But now, at least for the two cases touched here, only this ordering will be produced:
```
INSERT Y -> Z
INSERT X -> Y
```
meaning we build the tree from the bottom-up (the "bottom" being the root) and do out-of-order inserts less frequently.
Again, the biggest part of this diff should be readability/refactoring/de-duplicating logic, but more consistent orderings is a nice-to-have.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28017926
fbshipit-source-id: 5941588d0c8bba8b0df7d0084d5d198f4b7c2427
Summary:
Unit test case seed 1167342011 encodes a case where the differ produces a DELETE and CREATE of the same node in the same frame, which we consider an error.
It turns out this was caused by nested "unflatten" operations and this bit of deleted code specifically. We were deleting an unmatched node from a parent call's dictionary of nodes,
which prevented it from being matched in the "old" tree later on.
This is only possible now that we attach pointers to the "other" ViewNodePair when they're matched, so we can check existence of that pointer instead of inclusion in dictionaries to decide if we need to DELETE/CREATE a node and its subtree.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003330
fbshipit-source-id: 305440ef20b921883c1d6e38a4a4072e5a7f95ac
Summary:
Just adding a comment for future possible refactoring here.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003338
fbshipit-source-id: ec307314d18d69f8c77c2b2afff1f3953ca55473
Summary:
These blocks either are not necessary due to other mechanisms, or are impossible to hit.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003336
fbshipit-source-id: f2321073de77c0f0173a9a0891be2a3012578b01
Summary:
Since each ShadowViewNodePair will point to any matched pair in the "other" tree during diffing, we can rely on the presence of the "other" pointer instead of
always removing nodes from `deletionCreationCandidatePairs` when they're matched.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003335
fbshipit-source-id: 0b886946eedc497091ca79c436f160b3d4bf3f1e
Summary:
There's a lot of code duplication in the differ. Reduce by factoring a duplicated code path into `updateMatchedPairSubtrees`.
This handles cases of updating trees with flattening or unflattening.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003339
fbshipit-source-id: cbbf890ba447b29d79aedea374b173de40e71334
Summary:
Simple refactor to use this struct to store lists instead of references to lists.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003337
fbshipit-source-id: a37fa23ed3c1e1b273f92bf5ad5179a0fd1d852b
Summary:
Found by running random tests and extracting a failing seed.
This error existed in master (and existed prior to recent refactors) and will be fixed by the end of this stack.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003332
fbshipit-source-id: 9c4a10d236c24337b089c44e8c1beb22358cfb05
Summary:
This code can be uncommented locally and run several times to discover new failing seeds.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003333
fbshipit-source-id: 6a3b6c08ae02bccc5c4d26067409ff6c736f8a89
Summary:
Calling `FAIL()` doesn't flush glog, but `react_native_assert(false)` does. Both will have the effect of failing the test.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28003334
fbshipit-source-id: 802ad1f59e46eb048fd6ca95f5978eeaaad83f3a
Summary:
Changelog: [internal]
Current implementation of `AsynchronousEventBeat` dispatches lambdas through `RuntimeExecutor` regardless if it has done so previously.
So if `AsynchronousEventBeat::induce` is called 30 times, it will dispatch 30 lambdas.
In `AsynchronousEventBeatV2`, we make sure only single lambda is dispatched to `RuntimeExecutor` at a time.
Reviewed By: mdvacca
Differential Revision: D27940300
fbshipit-source-id: 2bad25c86315c1712b4a1da8c1d4702734cec70f
Summary:
Changelog: [internal]
EventPriority is backed by int, passing it by reference doesn't provide any performance benefits. Quite contrary, it can make it slower because of indirectness (in our case it is probably negligible).
Reviewed By: mdvacca
Differential Revision: D27938600
fbshipit-source-id: 37d1312627dd5a8f9012dfb35d21afe716a16ad7
Summary:
Fix a bug in the constructor of `LineMeasurement` where it did not initialise `xHeight` correctly.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D27972942
fbshipit-source-id: a56d55fdfe286bd11a6a81a3d024504f070bdb19
Summary:
This diff replaces all usages of int by int32_t. This is to ensure we always use a fixed size for int that matches what's expected on Java.
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27915608
fbshipit-source-id: 634c45796dda1d4434c3ad6ff3e199931c22940b
Summary:
There was a subtle bug in MapBufferBuilder::ensureDynamicDataSpace where in some situations allowed MapBufferBuilder to use unowned memory.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D27904642
fbshipit-source-id: de447633349094b0e8c5cb5e377dd31f5bfd1471
Summary:
Refactor MapBufferBuilder to use int to store size of Mapbuffer data
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D27904646
fbshipit-source-id: 6b8b96fdd30184b6d35c1d612743eae653854d6d
Summary:
DynamicData can contain a big amount of data, refactoring type to use int instead of short
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27904643
fbshipit-source-id: 157064b280e27a9c7c4a4f55af310392b178feda
Summary:
Dynamicdata can contain long datastructures, increasing type of MapBufferBuilder.dynamicDataSize to int
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27904647
fbshipit-source-id: ccbccf9328a6aa301aa3f9bf5c1b3c20f56e2a19
Summary:
found a bug in MapBuffer.getString() method, this diff is fixing it
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27904644
fbshipit-source-id: 746812235539ff75c5ce53c6f872ede9779fa1fa
Summary:
Add TODOs and Tasks into TODOs
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27865470
fbshipit-source-id: fcc7f86b0e6b290bf9f691a2dafea18d8addf782
Summary:
This diff adds an assert in the constructor of MapBuffer to ensure that we always access valid memory
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27864310
fbshipit-source-id: bd3870b6df1dbc5181fc5d852eb0ccbc32a8a951
Summary:
EZ refactor of MapBuffer::getCount and removal of TODO
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27864307
fbshipit-source-id: 14138090b3f2b45de8e0a3941abec990edb427ed
Summary:
This diff adds an assertion when trying to build a MapBuffer with invalid data
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27864309
fbshipit-source-id: 6601388e56be18ded0675f92cce009a577828c16
Summary:
Changelog: [internal]
Explicitly return undefined for `$$typeof` inside RuntimeSchedulerBinding.
React calls `$$typeof` on RuntimeScheduler. To make sure assert only triggers if unsupported value is requested, it needs to be explicitly handled
Reviewed By: mdvacca
Differential Revision: D27854472
fbshipit-source-id: 515c68d92b291cc274f5370c45e49302534e6f9c
Summary:
Changelog: [internal]
Scheduler's callback have option to add more work inside callback. This work stays on top of the priority queue and gives React ability to flush all work synchronously if need.
This diff adds use of `shouldYield_` to the workLoop. For now, it always evaluates to false. In the future when we allow access to the scheduler to native, it will allow yielding.
Relevant code in JavaScript:
https://github.com/facebook/react/blob/master/packages/scheduler/src/forks/SchedulerNoDOM.js#L190
Reviewed By: mdvacca
Differential Revision: D27823528
fbshipit-source-id: 016101e41eb7c41c2ac5abb55f803814867b8517
Summary:
changelog: [internal]
Callback function in React is expecting a boolean parameter indicating whether the callback timed out. React team is removing it, that's why we only pass in hardcoded false.
Reviewed By: mdvacca
Differential Revision: D27794562
fbshipit-source-id: b96a7b31560524b8f38ad3bb0dbdb3b3b32ac97b
Summary:
Changelog: [internal]
This brings native scheduler close to React's scheduler. React's scheduler executes all tasks in the queue within a single dispatch (they use setTimeout(0) for dispatch) and makes sure to not schedule two dispatches.
Relevant JS code:
https://github.com/facebook/react/blob/master/packages/scheduler/src/forks/SchedulerNoDOM.js#L359
Reviewed By: JoshuaGross
Differential Revision: D27793200
fbshipit-source-id: 4af13d95cfe4d33d0945f25929ccbea5f9ce5710
Summary:
CocoaPods will display a "fatal: not a git repository" when these podspecs are consumed within Facebook's internal Mercurial repository due to the reliance on `git` to obtain the current commit hash.
In these cases, the podspec is being consumed locally and the commit hash is unnecessary.
The error is removed by avoiding the use of `git` if the current working directory is not a git repository (or any of the parent directories).
Changelog:
[Internal] [iOS] - Remove CocoaPods error within Facebook's repository
Reviewed By: fkgozali
Differential Revision: D27750974
fbshipit-source-id: 99159611c580baf5526f116948c5ff60e1c02e5c
Summary:
I had intended to make this change as part of the stack I landed earlier, but I had some poorly resolved merge conflicts that left this path disabled.
I verified that T76057501 no longer repros and ran unit tests.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27788467
fbshipit-source-id: 42148b887c6b3c0e815f1805e6bfb3ee58503e48
Summary:
Add mounting layer test that stress-tests differ on (un)flattening.
This fails before D27759380 and D27730952, and passes after.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27767219
fbshipit-source-id: a7e186e510f95792da6f98f80fcae5ff8ac74775
Summary:
While I think this was a very marginal bug with no known issues in the wild, incorrect layout values were sometimes being propagated to certain nodes. This would only occur during complex nested (un)flattening operations and may only impact node consistency, specifically with setting the "previous" ShadowView of mutation instructions, specifically REMOVE and DELETE. Even in rigorous testing I had trouble hitting this case and it didn't seem to impact the "next" values in CREATE, INSERT, or UPDATE.
The issue: previously `sliceChildShadowNodeViewPairsV2` assumed that the node it's operating on is a child of a non-flattened view, and the baseline origin is `{0,0}`. You can see when `sliceChildShadowNodeViewPairsRecursivelyV2` is called, a `layoutOffset` is passed in. If we ever got a list of a node that was in a flattened parent by calling `sliceChildShadowNodeViewPairsV2`, we would incorrectly assume that baseline layoutOffset for the node is `0,0`.
Now, we store the layoutOffset in the ShadowViewNodePair and can retrieve it when getting child pairs of a node.
Changelog: [internal]
Reviewed By: sammy-SC
Differential Revision: D27759380
fbshipit-source-id: a89756190a1cb377bcc55ff31799c2afbaecdaa9
Summary:
Previously, `ShadowViewNodePair::List` owned each `ShadowViewNodePair` but whenever we put `ShadowViewNodePair` into a TinyMap, those were unowned pointer references. This worked... 99% of the time. But in some marginal cases, it would cause dangling pointers, leading to difficult-to-track-down issues. So, I'm moving both of these to be unowned pointers and keeping a `std::deque` that owns all `ShadowViewNodePair`s and is itself owned by the main differ function. See comments for more implementation details. I'm moderately concerned about memory usage regressions, but practically speaking this will contain many items when a tree is created for the first time, and then very few items after that (space complexity should be similar to `O(n)` where `n` is the number of changed nodes after the last diff).
See comments as to why I believe `std::deque` is the right choice. Long-term there might be data-structures that are even more optimal, but std::deque has the right tradeoffs compared to other built-in STL structures like std::list and std::vector, and is probably better than std::forward_list too. Long-term we may want a custom data-structure that fits our needs exactly, but std::deque comes close and is possibly optimal.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27730952
fbshipit-source-id: 2194b535439bd309803a221188da5db75242005a
Summary:
I am fixing an extremely marginal case that probably impacts nothing in production, but in theory, could - see next diff in stack for the assert that is being hit.
TL;DR in marginal, complex cases with a lot of un/flattening, we can generate the following sequence of mutations:
```
UPDATE node V1 -> V2
REMOVE node V1
```
That is incorrect, and what we actually want is:
```
UPDATE node V1 -> V2
REMOVE node V2
```
While this, again, impacts /nothing/ in prod that we know of, it would be good to get this correct so that we can enable stricter asserts (see next diff).
This will also help with debugging LayoutAnimations.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27697788
fbshipit-source-id: 47f34fe3e8107167b3df4db841d2cc14c58cb31d
Peter Argany
Samuel Susla
Joshua Gross
Neil Dhar
David Vacca
Héctor Ramos
CodemodService FBSourceClangFormatLinterBot