diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp index 126705685bc..e6f58b78b84 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -287,7 +287,11 @@ void ShadowNode::setMounted(bool mounted) const { family_->eventEmitter_->setEnabled(mounted); } -void ShadowNode::progressStateIfNecessary() { +bool ShadowNode::getHasBeenMounted() const { + return hasBeenMounted_; +} + +bool ShadowNode::progressStateIfNecessary() { if (!hasBeenMounted_ && state_) { ensureUnsealed(); auto mostRecentState = family_->getMostRecentStateIfObsolete(*state_); @@ -297,8 +301,10 @@ void ShadowNode::progressStateIfNecessary() { // Must call ComponentDescriptor::adopt to trigger any side effect // state may have. E.g. adjusting padding. componentDescriptor.adopt(*this); + return true; } } + return false; } const ShadowNodeFamily& ShadowNode::getFamily() const { diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h index 06b9d68f52e..b05831575f4 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h @@ -171,14 +171,22 @@ class ShadowNode : public Sealable, */ void setMounted(bool mounted) const; + /* + * Returns true if the shadow node has been marked as mounted before by + * calling `setMounted`. + */ + bool getHasBeenMounted() const; + /* * Applies the most recent state to the ShadowNode if following conditions are * met: * - ShadowNode has a state. * - ShadowNode has not been mounted before. * - ShadowNode's current state is obsolete. + * + * Returns true if the state was applied, false otherwise. */ - void progressStateIfNecessary(); + bool progressStateIfNecessary(); #pragma mark - DebugStringConvertible diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 1461f894b93..fd317991511 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -25,25 +25,50 @@ namespace facebook::react { using CommitStatus = ShadowTree::CommitStatus; using CommitMode = ShadowTree::CommitMode; -// --- Clone-less progress state algorithm --- +// --- State Alignment Mechanism algorithm --- // Note: Ideally, we don't have to const_cast but our use of constness in // C++ is overly restrictive. We do const_cast here but the only place where // we change ShadowNode is by calling `ShadowNode::progressStateIfNecessary` // where checks are in place to avoid manipulating a sealed ShadowNode. -static void progressStateIfNecessary(ShadowNode& newShadowNode) { - newShadowNode.progressStateIfNecessary(); +static void progressStateIfNecessary( + ShadowNode& newShadowNode, + const ShadowNode& baseShadowNode); - for (const auto& childNode : newShadowNode.getChildren()) { - progressStateIfNecessary(const_cast(*childNode)); +/* + * Looks at the new parent, new child and base child node to determine how to + * reconcile the state. + * + * Only to be called when baseChildNode has trait `ClonedByNativeStateUpdate`. + */ +static void progressStateIfNecessary( + ShadowNode& newShadowNode, + const ShadowNode& newChildNode, + const ShadowNode& baseChildNode, + size_t suggestedIndex) { + auto& shadowNode = const_cast(newChildNode); + if (shadowNode.progressStateIfNecessary()) { + // State was progressed without the need to clone. + // We are done with this node, but need to keep traversing. + progressStateIfNecessary(shadowNode, baseChildNode); + } else if (newChildNode.getHasBeenMounted()) { + // `newShadowNode` was cloned from react and cloned from a native state + // update. This child node was cloned only from a native state update. + // This is branching and it is safe to promote the new branch from + // native state update. + auto clonedChildNode = baseChildNode.clone({}); + newShadowNode.replaceChild(newChildNode, clonedChildNode, suggestedIndex); + } else { + // `newShadowNode` was cloned from react and cloned from a native state + // update. This child node was cloned also by react. + // we can't reason about this on this layer and need to keep traversing. + progressStateIfNecessary(shadowNode, baseChildNode); } } static void progressStateIfNecessary( ShadowNode& newShadowNode, const ShadowNode& baseShadowNode) { - newShadowNode.progressStateIfNecessary(); - auto& newChildren = newShadowNode.getChildren(); auto& baseChildren = baseShadowNode.getChildren(); @@ -63,21 +88,51 @@ static void progressStateIfNecessary( } if (!ShadowNode::sameFamily(newChildNode, baseChildNode)) { - // The nodes are not of the same family. Tree hierarchy has changed - // and we have to fall back to full sub-tree traversal from this point on. + // React has changed the structure of the tree. We will realign the + // structure below. break; } - progressStateIfNecessary( - const_cast(newChildNode), baseChildNode); + if (!baseChildNode.getTraits().check( + ShadowNodeTraits::Trait::ClonedByNativeStateUpdate)) { + // was not cloned with a new state, we can continue. + continue; + } + + progressStateIfNecessary(newShadowNode, newChildNode, baseChildNode, index); } + // === Realigning the tree === + + auto unprocessedBaseChildren = baseChildren.begin(); + std::advance(unprocessedBaseChildren, index); for (; index < newChildrenSize; ++index) { const auto& newChildNode = *newChildren[index]; - progressStateIfNecessary(const_cast(newChildNode)); + auto baseChildNodeIterator = std::find_if( + unprocessedBaseChildren, + baseChildren.end(), + [&newChildNode](auto baseChildNode) { + return ShadowNode::sameFamily(newChildNode, *baseChildNode); + }); + if (baseChildNodeIterator == baseChildren.end()) { + // This must never happen and there is a mismatch between the two trees. + // No way of recover from this, let's just continue. + continue; + } + + auto const& baseChildNode = *(*baseChildNodeIterator); + + if (!baseChildNode.getTraits().check( + ShadowNodeTraits::Trait::ClonedByNativeStateUpdate)) { + // was not cloned with a new state, we can continue. + continue; + } + + progressStateIfNecessary(newShadowNode, newChildNode, baseChildNode, index); } } -// --- End of Clone-less progress state algorithm --- + +// --- End of State Alignment Mechanism algorithm --- /* * Generates (possibly) a new tree where all nodes with non-obsolete `State` diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp index a25a60308f9..1ef49950a95 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp @@ -201,13 +201,11 @@ TEST_P(StateReconciliationTest, testStateReconciliation) { }, {.enableStateReconciliation = true}); - if (!GetParam()) { - EXPECT_EQ( - findDescendantNode(shadowTree, scrollViewFamily) - ->getState() - ->getRevision(), - state3->getRevision()); - } + EXPECT_EQ( + findDescendantNode(shadowTree, scrollViewFamily) + ->getState() + ->getRevision(), + state3->getRevision()); } TEST_P(StateReconciliationTest, testCloneslessStateReconciliationDoesntClone) { @@ -672,10 +670,7 @@ TEST_P(StateReconciliationTest, testScrollViewWithComplexChildrenReorder) { EXPECT_NE(findDescendantNode(shadowTree, childA->getFamily()), nullptr); EXPECT_NE(findDescendantNode(shadowTree, childB->getFamily()), nullptr); - if (!GetParam()) { - EXPECT_EQ( - findDescendantNode(shadowTree, childAFamily)->getState(), newState); - } + EXPECT_EQ(findDescendantNode(shadowTree, childAFamily)->getState(), newState); } TEST_P(StateReconciliationTest, testScrollViewWithChildrenReorder) {