Summary:
This implements the `MeasurableYogaNode` trait in `YogaLayoutableShadowNode`. We had this trait from the very beginning but never used it. Now, if the trait is specified, `YogaLayoutableShadowNode` will set the measure function for the node and dirty it during cloning.
Previously, we used (and still use) a dedicated method for setting up the measure function - `YogaLayoutableShadowNode::enableMeasurement()`. The problem with it is that to make it work we have to dirty the Yoga node every time we clone it. And the only proper way to do this in the `YogaLayoutableShadowNode` constructor because if we do it later ancestor nodes could not observe this and react to this. Therefore we have to have a trait for it.
The plan is to use it for TextInput first to fix a crash (see the next diff). After we confirm it works fine, we will replace all the usages of `enableMeasurement` with the new trait.
This diff also renames the other two yoga-related traits (to make them less verbose and look unified), adds more comments, and asserts.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25937711
fbshipit-source-id: fafbd5d62537ac09e02ffbfd56adab6d629d791d
Summary:
The purpose of this diff is to ensure that visibility changes are handled correctly when the value of "display" for a View changes from 'flex' to 'none'.
RNTester is nesting several Views with different kind of visibilities. When the user tap on an item there's a state update that changes the visibility styles for some of these views. Fabric does not reflect the right changes of visibility on the screen.
changelog: internal
Reviewed By: shergin
Differential Revision: D25841763
fbshipit-source-id: 769b97afb72939d346a4c6f2669ff938b35596bc
Summary:
Changelog: [internal]
EventQueue is used as a virtual class, this diff makes it one.
Reviewed By: JoshuaGross, shergin
Differential Revision: D25826983
fbshipit-source-id: 60e6937514cd3b837b0ca9f61bfaa081823ffc61
Summary:
Changelog: [internal]
Old event coalescing isn't used anymore and there haven't been any problems with the new one.
Reviewed By: shergin
Differential Revision: D25701311
fbshipit-source-id: 359f0361edffa22130cfa8322038acdbe26fd599
Summary:
In Android, only changed prop values are sent to the mounting layer via folly::dynamic maps. In the LayoutAnimation system, before this, we only sent that
map at the /end/ of the animation for any non-interpolated values (for example, image source is not interpolated so it was not updated until the end of the animation).
However, what we probably expect is that all non-interpolated values change immediately, and interpolated values smoothly transition. This diff makes that change on Android
by using the final RawProps as the /initial/ value that interpolations are stacked on.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25727483
fbshipit-source-id: e692d37b9965fedcdf429a81d60b7cb7f0c6abe1
Summary:
This replaces the internal core implementation of `setState` with the new `updateStateWithAutorepeat` which is now the only option.
In short, `updateStateWithAutorepeat` works as `setState` with the following features:
* The state update might be performed several times until it succeeds.
* The callback is being called on every retry with actual previous data provided (can be different on every call).
* In case of a static value is provided (simple case, not lambda, the only case on Android for now), the same *new*/provided value will be used for all state updates. In this case, the state update cannot fail.
* If a callback is provided, the update operation can be canceled via returning `nullptr` from the callback.
This diff removes all mentions of the previous state update approach from the core; some other leftovers will be removed separatly.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25695600
fbshipit-source-id: 14b3d4bad7ee69e024a9b0b9fc018f7d58bf060c
Summary:
ConcreteStateTeller is being replaced with a new built-in state autorepeat mechanism.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25687692
fbshipit-source-id: a89d0a4d63b9c97775a312afa3df43f26b5ecc08
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 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:
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]
Components can update state multiple times before the state update queue is flushed. This causes unnecessary layout/diff and mount passes. To solve this, drop stale state updates inside `stateUpdateQueue_ ` for specific `ShadowNodeFamily`.
Delivering stale status updates is redundant. Let's take SafeAreaView as an example. It schedules 5-6 state updates before `stateUpdateQueue_` is flushed. That's unnecessary work blocking JS thread. We only care about the latest state update. Same for TextInput and other components using state updates.
Reviewed By: JoshuaGross
Differential Revision: D23987707
fbshipit-source-id: 2e3f92cc93af61d78ac564aa40aef165af64b8c1
Summary:
Changelog: [internal]
`LayoutableShadowNode::findNodeAtPoint` was iterating children in incorrect order and wasn't taking zIndex into accout.
Reviewed By: JoshuaGross
Differential Revision: D23814866
fbshipit-source-id: 38eee297147a5c5912304d139bb10f8b16ae2ee1
Summary:
Exceptions in C++ work quite differently from exceptions in other languages. To make exceptions actually work **correctly** all the code needs to be written with "exceptions in mind" (e.g., see https://www.stroustrup.com/except.pdf). In short, if the code is not "exceptions ready", throwing an exception causes memory leaks, dangling pointers, and invariant violations all over the place, which will probably cause another crashes down the road (which will be especially hard to investigate and attribute to the original issue).
Fabric Core (Layout, Props parsing, ShadowNodes management, and so on) does not use exceptions because in most (all?) the cases the exception is now recoverable. So, if a program detects some internal state invariant violation or missing some resource, *logically* it's fatal. We also don't want to pay code-size and performance tax for exception support, so that's why we don't use them. It's just not the right fit for Fabric Core.
This does not mean that exceptions don't happen though. C++ standard library can throw them... sometimes. And if our library is compiled with exceptions enabled (still the case, unfortunately), an exception can bubble to JavaScript code and losing all context down the road. And it's hard to investigate such crashes. To isolate those occasional exceptions inside C++ core we are marking all C++/JS boundaries with `noexcept` that stops the bubbling.
I hope that will give us much more informative crash reports.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23787492
fbshipit-source-id: 0822dbf36fc680c15b02b5cd0f2d87328296b642
Summary:
This is *another* attempt to solve a failed state update problem.
Unfortunately, some applications are inherently not compatible with the "let's recommit the update on the application side in case it failed" approach. The problem is that if we call `updateState` on the application side, we miss the original event window. E.g. if we need to deliver some state update with AsyncBatched priority and if the update fails, we lose the opportunity to commit it on time. These issues can be critical for some complex use-cases as ComponentKit interop.
This diff adds implementation for `updateState` that does the work a bit differently. For all failed state updates it tres to recommit them asap using `ShadowTree::commit` and calling lambda on every attempt. With this approach the update might fail in two cases:
The node disappeared from the tree, so there is no way to update it.
The lambda returned `nullptr` indicating that the update is no longer needed.
We need this for the ComponentKit interoperability layer that is very sensitive for missing state updates.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23603958
fbshipit-source-id: a3b8c09fb2f1c8302583aa5880b48fc0840224e3
Summary:
This pull request changes the default constructor in the `TestProps` class in `react/renderer/core/tests/TestComponent.h`
`using ViewProps::ViewProps;` was causing problems in Visual Studio 2017 - changing it to ` TestProps() = default;` allowed dependent CPP files to compile on MSVC and caused no regressions in the Fabric test suite.
## Changelog
Changelog: [Internal] [Changed] - Fabric Tests: Change default constructor in TestComponents' TestProps
Pull Request resolved: https://github.com/facebook/react-native/pull/29898
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23591986
Pulled By: shergin
fbshipit-source-id: 132e1c2e38fa74aa4f2c8746054d6152f30035e9
Summary:
Changelog: [internal]
# What is Teller?
Teller is a bank's employee who deals with the customer on behalf of the bank. In Fabric's scenario it is a class that on behalf of the view deals with State.
# Why do we need it?
Dealing with `ConcreteState` can be complicated and patterns are often repeated among different component views. `ConcreteStateTeller` aims to resolve these issues. Examples:
- You can call teller's methods without checking for nullptr (we have had crashes because of this before).
- Methods are save to be called on any thread.
- Mechanism to retry state update if it fails is built in.
It is designed to be used from ComponentView so views don't have to talk directly to `ConcreteState`.
Reviewed By: JoshuaGross, shergin
Differential Revision: D23216865
fbshipit-source-id: 90a50702e036eac084f89743ebab687a67182dc0
Summary:
Changelog: [Internal]
If `ConcreteShadowNode::setStateData` is called and the node is cloned before it is mounted, the cloned node will have old state before `setStateData` was called.
To solve this, simply call `setMostRecentState` on the family inside `ConcreteShadowNode::setStateData`.
Reviewed By: JoshuaGross
Differential Revision: D23283560
fbshipit-source-id: f9822fb69e4234f776d512fc02fe13ea7de64897
Summary:
There are a few places where we cast JSI values to objects without much validation and without proper error logging, and in some places the crashes aren't symbolicated well. To make debugging easier in the short-term, I'm adding some additional logs.
Changelog: [Internal]
Reviewed By: mdvacca, RSNara
Differential Revision: D23033222
fbshipit-source-id: 9343d693a441f0af728e560a0c245bcc4eb97869
Summary:
Changelog: [Internal]
Fabric's UIManager.measureInWindow didn't take viewport's offset into account. This diff fixes it by including viewport's offset in `LayoutContext`.
Reviewed By: JoshuaGross
Differential Revision: D23021903
fbshipit-source-id: 9106a8789d66fe19d8cb0a9378ee5bc8f2c83005
Summary:
Changelog: [internal]
Use `Element<>` in `LayoutableShadowNodeTests`. It makes the tests cleaner and easier to understand.
Reviewed By: JoshuaGross
Differential Revision: D23028341
fbshipit-source-id: f7a2255581bdde667db0f68c222228a5b405b22f
Summary:
Changelog: [Internal]
# Problem
## Step 1
JS clones a node that has size {100, 100} and changes props that cause the node to increase size to {200, 200}. JS holds pointer to this node.
Now, the size (stored in LayoutableShadowNode.layoutMetrics_) changes after Yoga layout is triggered.
However, the node gets cloned inside State Reconciliation before Yoga layout phase. The JS pointer points to a node with size {100, 100}, not to a node with size {200, 200}.
## Step 2
Again, JS clones node (with old reference, therefore gets old layoutMetrics_ with size {100, 100}) and it changes props that cause the node to decrease its size back to {100, 100}.
We go all the way to Yoga layout and looking for nodes that have been affected by the node. The node, affected by the layout because it went from {200, 200} to {100, 100}, will be evaluated as not affected. This causes onLayout event to not be fired.
# Fix
We can safely remove the frame equality check (please see below). This can be done because we already check for equality before dispatching onLayout. It happens here:
https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/js/react-native-github/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp?commit=881853eb0c42625fd0812bd2652bf36fcbd614ee&lines=43
As far as I know, `affectedNodes` isn't used for anything else besides dispatching onLayout.
# Discussion
This problem manifests itself only when a node has two different sizes that it flips between. To better understand this, please watch the video in Test plan labelled "before". Notice how the text has 2 different values that it flips between.
Here is a code that was affected by it https://fburl.com/diffusion/3hwo0iy5
If you inspect it closely, you will notice that it depends on `onLayout` to return correct value to calculate offset from left.
Reviewed By: JoshuaGross
Differential Revision: D22999891
fbshipit-source-id: e2d0f5771c1bf3cd788e5e9da0155c92e33fb84e
Summary:
Make react/core module to compile in OSS
This is necessary to make fabric compile in OSS
changelog: [internal] internal
Reviewed By: fkgozali
Differential Revision: D22908222
fbshipit-source-id: a37b87d02ecf77bb25693ce32cd0f3432be5daa7
Summary:
This diff creates the Android OSS build system for the module react/utils
As part of this diff I also moved the module to react/utils folder
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D22877265
fbshipit-source-id: 717487aacb392d0f08530763a16a638b8021d501
Summary:
iOS will need to be implemented separately, but the shared C++ bits are in place.
Explanation: there is currently no way for the View layer to /know/ if an UpdateState call has succeeded or failed. Generally we just assume it succeeds, but if it fails we have no way of knowing or retrying.
This can cause some UI bugs. To mitigate this, I'm introducing a "failure" notification callback mechanism. The JNI bridging for this is a little complicated to avoid passing Runnable across the JNI, but it
should be much simpler on iOS.
In development this seems to make View components much more reliable.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22940187
fbshipit-source-id: 917f2932ae22d421f91fe8f4fca3f07dc089f820
Summary:
This diff removes the inner folder of react/renderer/core, moving all its files into react/renderer/core
This is necessary to simplify the compilation of Fabric in OSS
More details: https://fb.quip.com/amaRA631DX3K
changelog: [internal] Internal
Reviewed By: fkgozali, JoshuaGross
Differential Revision: D22875854
fbshipit-source-id: e2d969c3ec67eab1bbdc9288e5a4285c740fa944
Summary:
This diff moves fabric C++ code from ReactCommon/fabric to ReactCommon/react/renderer
As part of this diff I also refactored components, codegen and callsites on CatalystApp, FB4A and venice
Script: P137350694
changelog: [internal] internal refactor
Reviewed By: fkgozali
Differential Revision: D22852139
fbshipit-source-id: f85310ba858b6afd81abfd9cbe6d70b28eca7415
Valentin Shergin
David Vacca
Andres Suarez
Samuel Susla
Joshua Gross
empyrical