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
empyrical
Samuel Susla
Joshua Gross
David Vacca