Summary:
Changelog: [internal]
Touch events should be of a type PressEvent. Fabric only provided touches, changedTouches and targetTouches and leaves out force, identifier, locationX, locationY, pageX, pageY and timestamp.
Reviewed By: shergin
Differential Revision: D25243347
fbshipit-source-id: e824558bd43f51c0c6dcca62bfc98318aa61678e
Summary:
Changelog: [internal]
This is already defined in Touch.h
Reviewed By: shergin
Differential Revision: D25242843
fbshipit-source-id: 23bac2a60f3d995e34d342c3a189760875f4bc77
Summary:
Changelog: [internal]
Return value from `LayoutAnimationKeyFrameManager::getAndEraseConflictingAnimations` was a tuple with 3 elements. Two of them are not being used so let's get rid of them.
Reviewed By: JoshuaGross
Differential Revision: D25220601
fbshipit-source-id: 35781e735b6a2e518337fdeaf956c18bb370993b
Summary:
Changelog: [internal]
The assert is still firing, let's disable it until we can investigate why layout animations creates two delete mount instructions.
Reviewed By: majak
Differential Revision: D25216794
fbshipit-source-id: 6328a2afb5eaf7fceebdc05bc75804f2eb44ddd2
Summary:
These configs are never actually empty, so they shouldn't be optionals.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D25129254
fbshipit-source-id: 626119fefad0440732541c680286ebbbfab6aeba
Summary:
Changelog: [internal]
`currentAnimation_` is accessed on multiple threads and has dedicated mutex, but it was not acquiring the mutex in `LayoutAnimationKeyFrameManager::shouldAnimateFrame`
Reviewed By: JoshuaGross
Differential Revision: D25121654
fbshipit-source-id: 38b1c82eaabab283beab18dc210ea21379edbe93
Summary:
Changelog: [Internal]
Contents of `callComplete_` are accessed from JavaScript thread and main thread and there is possible race condition. This diff changes `bool` to `atomic_bool` to prevent the race.
Reviewed By: JoshuaGross
Differential Revision: D25094907
fbshipit-source-id: 6a2c6e33ab5ba0c6ab728e175f2e5c11fdd0a579
Summary:
Changelog: [Internal]
General improvements. Behaviour should be exactly the same.
Reviewed By: JoshuaGross
Differential Revision: D25092505
fbshipit-source-id: 584640ece3e02d468f6bcb84577d7a6c899cc253
Summary:
Imagine the scenario in which there's an ongoing UPDATE animation; a REMOVE+INSERT (move) is queued up for the same tag, but there's no
new corresponding UPDATE - so maybe the indices of the view have changed, but the layout stays the same. Under the old model, the previous animation would be canceled and the node would jump to the final position. In theory, if there's no new UPDATE, we should continue animating the node to its final position.
I'm much happier with this - conflicting animations transition into each other super seamlessly now, and I think the logic is more straightforward as well.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D25071664
fbshipit-source-id: fcefc4619dc34cdafdc4d8e8e730b935e5528290
Summary:
This is noisy when enabled, and not very useful.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D25071584
fbshipit-source-id: 7205b5fa39622feccaf315ccebb181dbdac4281d
Summary:
See comments, hopefully they explain this situation. This fixes the last remaining case that I have repro'd where StubViewTree asserts fire.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D25062135
fbshipit-source-id: a28afba21f4094200aa0502b1e085dcbc10f9835
Summary:
The current implementation of LayoutAnimations assumed that the "previous/old" ShadowView passed into the diff mutation didn't matter except for purposes of diffing.
As it turns out, iOS components could possibly use the "old" version of props, state, etc - so we should try to keep track of the current value in the tree as much as possible.
This diff accomplishes that by keeping track of the "previous" view, which the AnimationDriver will update over time. This also allows us to simplify logic around conflicting animations.
I'm also adding a few additional asserts to assist in debugging.
This doesn't totally eliminate all asserts hit on iOS, yet, but it does reduce the number of times the asserts are hit in StubViewTree.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D25048644
fbshipit-source-id: d00aeece5af04624d8193063be453c7ce4a6e565
Summary:
Like the task mentions `strongSelf->_eventInterceptors` was crashing, probably because the coordinator was cleaned up before this block ran.
Check to make sure self is still valid before attempting to access any instance variables.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D25073812
fbshipit-source-id: cdf666f2ac028b5523097f15ff51fbae9f9ffbd8
Summary:
Changelog: [internal]
Reference here is incorrect, we need a container for `ShadowViewMutation`.
Reviewed By: JoshuaGross
Differential Revision: D25024080
fbshipit-source-id: f59a18d859ad391bc168c8990d40b25d18003f74
Summary:
Changelog: [internal]
Previous implementation of coalescing would only look at the last element in `eventQueue_` and if it was the same type and target, it would coalesce the two together. This was problem when user would scroll in UIScrollView, this triggers onTouchMove and onScroll events at high rates and prevents coalescing of them.
This changes changes the behaviour to search the `eventQueue_` backwards for an event of the same type and target. If one if found, it is moved into its place. If even of another type is found before for the same target, the event is pushed back onto the queue.
Reviewed By: JoshuaGross
Differential Revision: D24992941
fbshipit-source-id: fc1eae4ecd100af6202346674778b0634ed7a15b
Summary:
This diff adds more enforcement for consistency of `ShadowNodeMutation`s, including:
* `Props` object for newly created or updated view must not be nullptr;
* `oldShadowView` must describe the previous state of the view for `Update` instruction;
* `ignoreDuplicateCreates` option was removed.
I suspect some of the crashes we see in Fabric are caused by a violation of one of these constraints. If one of these fails in debug builds, we will get an early signal.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D24880821
fbshipit-source-id: 8c8a3d8e205ce34f6e0335e8a2b0cf676930c284
Summary:
This is the final diff in the JS TurboModule codegen stack for Android. It implements method dispatch using the TurboModuleSchema object.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D22837486
fbshipit-source-id: f91b03f064941457d4b8c5e37e011468559dee71
Summary:
JavaTurboModule will use instances of this class to perform method invocation. TurboModuleSchema is created by parsing the `jsi::Value` that represents the TurboModule's schema.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D22832729
fbshipit-source-id: 792736e176c33a302f6a41c6f63a4812b09af585
Summary:
## Android API
```
// Before we initialize TurboModuleManager
ReactFeatureFlags.useTurboModuleJSCodegen = true
```
## iOS API
```
// Before we initialize RCTBridge
RCTEnableTurboModuleJSCodegen(true);
```
## How is the JS Codegen actually enabled?
The above native flags are translated to the following global variable in JavaScript:
```
global.RN$JSTurboModuleCodegenEnabled = true;
```
Then, all our NativeModule specs are transpiled to contain this logic:
```
interface Foo extends TurboModule {
// ...
}
function __getModuleSchema() {
if (!global.RN$JSTurboModuleCodegenEnabled) {
return undefined;
}
// Return the schema of this spec.
return {...};
}
export default TurboModuleRegistry.get<Foo>('foo', __getModuleSchema());
```
Then, in our C++ JavaTurboModule, and ObjCTurboModule classes, we use the TurboModule JS codegen when the jsi::Object schema is provided from JavaScript in the TurboModuleRegistry.get call.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D24636307
fbshipit-source-id: 80dcd604cc1121b8a69df875bbfc87e9bb8e4814
Summary:
This change should make all type-safe NativeModules TurboModule-compatible.
Changelog: [Internal]
Differential Revision: D24729493
fbshipit-source-id: 7712708a24d675ca567225797016a7ff66a2920e
Summary:
This change introducing using `updateStateWithAutorepeat` for state updates in RCTSafeAreaViewComponentView. This way we can reduce the number of active commits and reduce jumps & relayout passes.
The approach with a callback is better than using `_lastPaddingStateWasUpdatedWith` because:
* When we compare the values, we can compare them with actual previous padding numbers stored in Shadow Tree.
* The value stored in a UIView instance can go away because of a view being remounted because of flattening.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D24719345
fbshipit-source-id: 9bf1ae284875b4c99cf23be2fcc9a829eb8a895e
Summary:
This implements a new ShadowNode trait that helps to propagate Yoga node `isDirty` flag down the root of the tree and clone siblings appropriately.
Several Fabric components mutate its Yoga styles after the node was cloned. In such cases, we need to mark the node as dirty after doing so. The problem with this is that the parent node and its siblings were already updated (cloned or not) based on the previous value of the `isDirty` flag. This happens because this logic is implemented in YogaLayoutableShadowNode which is a base constructor that must be called before any other logic from a subclass can run.
For now, this change enables that for SafeAreaView only (which seems to help with some junkiness issues), later we can extend the usage of this for other components if needed.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D24719347
fbshipit-source-id: b0d050afea5de9c470e05e1b4c9e7052e00ae949
Summary:
For some interrupted animations we will execute a "final" mutation associated with the animation, if it exists. For example, "UPDATE" animations always have a final Update animation associated with them.
Some, however, do not. For example: INSERT animations do not have a final mutation associated by default. In these cases (before this diff) if the animation from opacity 0 to 1 was interrupted, the View will
appear "stuck" at some intermediate opacity. To mitigate that, we generate a synthetic "final" mutation at 100% progress through the animation if it is interrupted.
Changelog: [Internal]
Reviewed By: fred2028
Differential Revision: D24691151
fbshipit-source-id: d9730b8a3493a5eeac4de325e7e0a7a64f73c8a0
Summary:
When shouldEnableLoggingForRequestUrl is false, ImageTelemetry is not initialized, and no logging is done.
* Replace `- (NSString *)loaderModuleNameForRequestUrl:(NSURL *)url` with `- (BOOL)shouldEnableLoggingForRequestUrl:(NSURL *)url`
* Rename RCTImageLoaderInstrumentableProtocol.h -> RCTImageLoaderLoggableProtocol.h
Reviewed By: fkgozali
Differential Revision: D24523984
fbshipit-source-id: a5463eceea1c40f9452b0ad2ee6bf047f71a02c1
Summary:
The callback `dispatchEvent` is called asynchronously on the JavaScript thread, so all data it uses must be copied to the lambda. To work around this constraint we use a shared pointer for the counter.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24598840
fbshipit-source-id: fb5581858d54dc806863caf0c7c4f612ed6046e2
Summary:
Changelog: [internal]
Background executor performs unnecessary operations when second `completeRoot` message from React arrives before first `completeRoot` was finished. This produces unnecessary `ShadowViewMutations`.
Mechanism:
Everytime `completeRoot` is received, before the call is dispatched on the background queue, `completeRootEventCounter_DO_NOT_USE_` is incremented.
Inside `ShadowTree::tryCommit` we check if the value has been incremented to determine if another `completeRoot` is queued.
Reviewed By: JoshuaGross
Differential Revision: D24419160
fbshipit-source-id: 11e19026feca01db6c8981b093a691a6b58a006f
Summary:
Changelog: [internal]
There are two exceptions inside `TelemetryController::pullTransaction`:
- Empty Optional cannot be unwrapped
- mutex lock failed: Invalid argument
By marking this method `noexcept`, stack trace is lost and it makes it more difficult to track down the issue.
What does compiler do if a method is marked `noexcept`?
```
void f() noexcept {
try {
// do work
}
catch (...) {
std::terminate(); // This is the std::terminate() we are seeing in stack traces.
}
}
```
Removing noexcept specifier might give us more information about the exception.
Reviewed By: JoshuaGross
Differential Revision: D24477861
fbshipit-source-id: 80f26e9ab160a5330c2848b89a01d60bfc0a4611
Summary:
Added a few FB vs OSS polyfills:
* react_native_root_target() to refer to the root FB react-native-github/ dir or repo dir in OSS
* react_native_xplat_synced_target() for anything xplat
* a few others
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D24437245
fbshipit-source-id: ee290a87a98a8e9be67b102a96f2faac2a2cb92b
Summary:
#changelog: [internal]
When I built ThreadStorage I didn't know about existence of `thread_local` keyword. Because it achieves the same goal, using built in c++ features is preferred over building our own.
Reviewed By: JoshuaGross, shergin
Differential Revision: D24380680
fbshipit-source-id: e961fc34c6d3f085fc9b918b20bb4827de0d5624
Summary:
changelog: [internal]
Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t
# Why is using size_t better when working with indexes.
## 1. Type conversion isn't for free.
Take this example
```
size_t calculate(int number) {
return number + 1;
}
```
It generates following assembly (generated with armv8-a clang 10.0.0):
```
calculate(int): // calculate(int)
sub sp, sp, #16 // =16
str w0, [sp, #12]
ldr w8, [sp, #12]
add w9, w8, #1 // =1
mov w8, w9
sxtw x0, w8
add sp, sp, #16 // =16
ret
```
That's 9 instructions.
If we get rid of type conversion:
```
size_t calculate(size_t number) {
return number + 1;
}
```
Assembly (generated with armv8-a clang 10.0.0):
```
calculate(unsigned long): // calculate(unsigned long)
sub sp, sp, #16 // =16
str x0, [sp, #8]
ldr x8, [sp, #8]
add x0, x8, #1 // =1
add sp, sp, #16 // =16
ret
```
Compiler now produces only 7 instructions.
## Semantics
When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.
Reviewed By: JoshuaGross
Differential Revision: D24332248
fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
Summary:
We will need it soon.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24290775
fbshipit-source-id: a312e537a3c3954e709a10c8792b3462b574054a
Summary:
Changelog: [internal]
Fabric uses view commands instead of setNativeProps. This diff removes what's left of setNativeProps from the core.
Reviewed By: JoshuaGross
Differential Revision: D24309999
fbshipit-source-id: 70e54f0a984f8c36f77ba2cd59f59fc6923bc832
Summary:
This diff ended up being a bit more complicated than I anticipated, since the source files in `ReactInternal` were depending on `RCTEventDispatcher`. I made the following changes:
1. Make `RCTEventDispatcher` a `protocol`, keep it in `ReactInternal`.
2. Rename the `RCTEventDispatcher` NativeModule to `RCTEventDispatcherModule`, make it conform to the `RCTEventEmitter` `protocol`, and move it to `CoreModules`.
3. Where necessary, replace categories of `RCTEventDispatcher` with functions.
Changelog:
[iOS][Added] - Make RCTEventDispatcher TurboModule-comaptible
Reviewed By: fkgozali
Differential Revision: D18439488
fbshipit-source-id: b3da15c29459fddf884519f33b0c3b8c036b5539
Summary:
There are a few places where we have JNI table ref leaks, and more places where we can aggressively clean up smart pointers immediately instead of waiting for them to be cleaned up at some later point.
In theory these smart pointers should be cleaned up immediately, but in cases where many components are being measured at once, the JNI table could grow until all measure calls are done. In extreme cases this
could cause a crash, which I want to avoid. At the very least, freeing memory more aggressively in this case can't hurt.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24293775
fbshipit-source-id: 159741ba955e5a6fe02caf6e65d1e4d6d4afadee
Summary:
This adds support for `<Picker style={}/>` prop for text styling. It reuses most of conversion logic from BaseText. This means that it actually supports more styles than Paper Picker supported. (Paper picker only supported ~4 styles, this supports everything that Text supports, so 10+ styles).
The only tricky thing is that Picker supports multiple ways of setting text color. Both
<Picker
itemStyle={{color: '#008BD0'}} >
<Picker.Item label="Java" value="java" />
<Picker.Item label="JavaScript" value="js" />
</Picker>
and
<Picker>
<Picker.Item label="Java" value="java" color={'#008BD0'} />
<Picker.Item label="JavaScript" value="js" />
</Picker>
technically work in Paper. I've decided to maintain this behaviour (since there's lots of product code callsites to both options).
Changelog: [iOS][Fabric] Fabric Picker support
Reviewed By: sammy-SC
Differential Revision: D23980319
fbshipit-source-id: e469a837e28af0ad97cf0e171df26ee19adff3ab
Summary:
This builds on previous diff to properly parse `<Picker.Item/>` into a cpp struct.
This fixes the dummy text and text color TODOs.
Changelog: [iOS][Fabric] Fabric Picker support
Reviewed By: sammy-SC
Differential Revision: D23964557
fbshipit-source-id: f42c6c9cf410bfc5e66ff078645b6378548481de
Summary:
This is a starting point for the handwritten Fabric Picker component. It is incomplete, and needs to be landed with the rest of the stack above it.
In general, this creates a new `ComponentView`, `ComponentDescriptor`, `ShadowNode`, `Props` and a few other boilerplate classes for Picker. A bunch of the logic in `ComponentView` was copied over from the Paper `RCTPicker` and `RCTPickerManager`.
What works in this diff:
- A `<Picker>` with items can be created in JS, and a corresponding `UIPicker` is created in native with placeholder text, default styling and the correct amount of items
What doesn't work yet (implemented in later diffs):
- Parsing items to use correct text and styling in native
- Events/commands
Reviewed By: sammy-SC
Differential Revision: D23941821
fbshipit-source-id: e049ca6004757fbd1361985644d5dbb8f53e1ce6
Summary:
## Previously
- When TurboModules system was on, we'd only log events from the TurboModules system.
- When TurboModules system was off, we'd only log events from the NativeModule system.
This ultimately gives us less data to analyze both systems in production.
## Changes in This Diff
When perf. logging is on, we'll log events from both systems. Each QPL event now include an annotation of which system the event is coming from. Concretely, this will allow us to see how much of the NativeModule system is being exercised in the TurboModule test group.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D24232594
fbshipit-source-id: 7dff57bd74fc7ef744d3e06ff174304f25790456
Samuel Susla
Joshua Gross
Peter Argany
Paige Sun
Valentin Shergin
Ramanpreet Nara
Pieter De Baets
Kevin Gozali
Lulu Wu