Summary:
This diff has the bridge attach RCTCallableJSModules to our NativeModules.
Changelog: [Internal]
Differential Revision: D28395447
fbshipit-source-id: 01ca62442013826d28ba0f710e28a5963f5efb65
Summary:
Changelog: [internal]
Only moves methods around so public and private methods are grouped together.
Reviewed By: JoshuaGross
Differential Revision: D28381588
fbshipit-source-id: dae7f493b99845f66070689270bc7aef958fbecd
Summary:
Changelog: [internal]
This diff moves all calls to RuntimeExecutor to RuntimeScheduler. The calls are still immediately dispatched. Timing of events will not change.
The goal of this diff is to prepare infrastructure for Concurrent Mode.
Reviewed By: JoshuaGross
Differential Revision: D27937665
fbshipit-source-id: 434d78c95ccf23d8da41186d0dae91bff4eda384
Summary:
react-native-windows runs with a more strict set of warnings as errors. This fixes a bunch of warnings being hit while compiling core react-native code as part of react-native-windows. In particular warnings about mismatched signed/unsigned comparisons, lossy conversions, and variable names that conflict with names in outer scopes (yoga has a global for `leading` and `trailing` that conflicts with some local variable names)
## Changelog
[Internal] [Fixed] - Fix various C++ warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/31399
Test Plan: I've run these changes in react-native-windows. -- Shouldn't have any functionality difference.
Reviewed By: sammy-SC
Differential Revision: D28290188
Pulled By: rozele
fbshipit-source-id: 2f7cf87f58d73a3f43510ac888dbcb9ab177d134
Summary:
Changelog: [internal]
Cloning root shadow node didn't always trigger a layout because node was not dirtied. Checking if layoutConstraints are changed when cloning RootShadowNode
Reviewed By: JoshuaGross
Differential Revision: D28352238
fbshipit-source-id: 536afdeddf5d2019950ceb6664c10ee593ab5f02
Summary:
Builds on previous diff. Instead of running a single animation to completion, queues up an animation and then spams mutations that could conflict with the animation.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D28357892
fbshipit-source-id: 6ca83a56dded629d8a1444360f1d02037661a5ef
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:
Changelog: [internal]
Prevent unnecessary copy of `RawEvent::eventTarget` which is `shared_ptr` inside `EventQueue::enqueueEvent` by passing in rvalue.
Reviewed By: JoshuaGross
Differential Revision: D28323219
fbshipit-source-id: 7f62e17df5c4264a15adf58f6142155a76de7aae
Summary:
Support ScrollAway in ReactScrollView for Fabric/non-Fabric.
Changelog: [Android][Added] Support for ScrollAway native nav bars added to ReactScrollView
Reviewed By: mdvacca
Differential Revision: D28308855
fbshipit-source-id: 9a922159ef50fb7c8e9c484a4b97ca57ab248496
Summary:
After this diff, every NativeModule that has `synthesize bundleManager = _bundleManager`, will get access to an RCTBundleManager, that it can use to read from/write to the bridge's bundle URL.
Changelog: [iOS][Added] - Attach RCTBundleManager to NativeModules
Reviewed By: PeteTheHeat
Differential Revision: D28086319
fbshipit-source-id: 6e4cd815d300e9036957ec8c743e947d2cb3f365
Summary:
Changelog:
[General][Added] Add support for "togglebutton" accessibilityRole
# Context
The role for ToggleButton, which is needed on Android to implement toggle buttons correctly, is not currently supported.
# What does this diff do?
Adds support for accessibilityRole `"togglebutton"`.
On Android, this maps to class `"Android.widget.ToggleButton"`.
iOS does not have an equivalent trait for togglebutton, so I set it to be the same as setting `accessibilityRole="button"` for iOS.
# Caveats - checked vs selected
It seems to me like this role currently requires that you set `accessibilityState={{checked: true/false}}`. The behavior is strange when setting `selected` state, I think because on Android ToggleButtons are meant to use `checked` to indicate toggled on/off.
This is tricky because typically on iOS if you have a toggle button, you would use `selected` instead of `checked`, so RN users are likely to mess this up.
Possible solutions:
1. document that you should use `checked` state on Android for toggle buttons (and maybe throw a warning if someone passes in `selected`).
2. have RN ignore it if someone passes in accessibilityState `selected`, if this role is used.
3. Have RN convert passed in `selected` state to `checked` on the Android side.
Reviewed By: nadiia
Differential Revision: D27976046
fbshipit-source-id: 4ce202449cf2371f4bf83c4db2d53120369ee7b0
Summary:
Recent changes to `MapBuffer` have broken the compilation on Windows.
This fix is similar to this recently-merged change: https://github.com/facebook/react-native/pull/31106
Side note - this PR only addresses a build break, but doesn't address the unsafe casting semantics in `MapBuffer`, which can still cause overflows.
## 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] - Fix compilation errors on Windows.
Pull Request resolved: https://github.com/facebook/react-native/pull/31363
Test Plan: RN now builds in Visual Studio on Windows.
Reviewed By: mdvacca
Differential Revision: D28028342
Pulled By: rozele
fbshipit-source-id: 77d8d4870c59b77acfc0ab2f4c3b7df40b59851d
Summary:
## Rationale
The TurboModuleManager should only be concerned with modules. The bridge and RCTInstance integrate the TurboModuleManager with the rest of React Native. Therefore, abstractions used by TurboModules that reach into the rest of React Native should be attached by the bridge or by RCTInstance.
## Changes
In this diff, we pull RCTViewRegistry attachment out of the TurboModuleManager. In bridge mode, it'll be attached to TurboModule by the bridge. In bridgeless mode, it'll be attached to TurboModules by the RCTInstance.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D28086320
fbshipit-source-id: 9d99835bdbb66bb6a41fbd0d8a3970cefae16b81
Summary:
When REMOVE animations finish, they are hitting asserts in StubViewTree because of UPDATEs and REMOVEs being out-of-order. By simply not forcing REMOVEs to happen before UPDATEs, this problem seems to go away.
Long-term (in a couple weeks, likely) I want to add unit tests to catch all cases like this, but I'm comfortable with this change since it does fix a known assert failure.
Changelog: [Internal]
Differential Revision: D28086223
fbshipit-source-id: eb77ea94d76e996d7b2cb37038ce6f2a9799f8b4
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:
All NativeModules/TurboModules can use the following Venice-compatible API instead:
```
synthesize moduleRegistry = _moduleRegistry
```
In bridge mode, it'll look up the module via the TurboModule system/bridge.
In bridgeless mode, it'll look up the module via the TurboModule system.
Therefore, there's no need to support this API.
Changelog: [iOS][Removed] - Delete synthesize turboModuleRegistry API in RCTTurboModule
Reviewed By: fkgozali
Differential Revision: D28070740
fbshipit-source-id: d2c8285fd4c05b67fb03ce82217bf6ddfd1dd685
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:
## Rationale
With the bridge, the invalidate method of every TurboModule gets called on the TurboModule's method queue. This ensures that we can safely clean up resources accessed in the TurboModule's async methods. In bridgeless mode, we don't invalidate each module on its method queue. After this diff, we will clean up every TurboModule on its own method queue.
## Changes
- Make RCTInstance dealloc itself on the JavaScript thread.
- Terminate the JavaScript thread on the JavaScript thread. This ensures that no work executes on the JavaScript thread after we clean up RCTInstance.
- Make [RCTTurboModuleManager invalidate] execute right before RCTInstance terminates the JavaScript thread.
- **Fix:** Make [RCTTurboModuleManger invalidate] synchronously invalidate modules on their own method queues.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D27933587
fbshipit-source-id: 7630e7fc074df2f5a3293192431105c747b8588f
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
Ramanpreet Nara
Samuel Susla
Andrew Coates
Joshua Gross
Kacie Bawiec
Igor Klemenski
Peter Argany
Neil Dhar
David Vacca