From 1183d82884cdb9ff31add7fff6c7ed202139f566 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 7 Sep 2018 21:44:18 -0700 Subject: [PATCH] Fabric: Removing `ShadowNode::operator==` Summary: @public We don't need this anymore. The same functionality is now implemented as `ShadowView::operator==` in much more reasonable way. Reviewed By: sahrens Differential Revision: D9649821 fbshipit-source-id: 8cd5f3cb4f583fd10d2d1e060aba914541341b5b --- .../components/view/ConcreteViewShadowNode.h | 11 ----------- .../fabric/core/shadownode/ShadowNode.cpp | 17 ----------------- ReactCommon/fabric/core/shadownode/ShadowNode.h | 12 ------------ .../fabric/core/tests/ShadowNodeTest.cpp | 6 +++--- ReactCommon/fabric/uimanager/Differentiator.cpp | 9 ++++++--- 5 files changed, 9 insertions(+), 46 deletions(-) diff --git a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h index 209a85940..ad908f545 100644 --- a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h @@ -113,17 +113,6 @@ public: return clonedChildShadowNode.get(); } -#pragma mark - Equality - - bool operator==(const ShadowNode& rhs) const override { - if (!ShadowNode::operator==(rhs)) { - return false; - } - - const auto &other = static_cast(rhs); - return getLayoutMetrics() == other.getLayoutMetrics(); - } - #pragma mark - DebugStringConvertible SharedDebugStringConvertibleList getDebugProps() const override { diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.cpp b/ReactCommon/fabric/core/shadownode/ShadowNode.cpp index 6310cab5f..bed59f615 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.cpp +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.cpp @@ -143,23 +143,6 @@ void ShadowNode::cloneChildrenIfShared() { children_ = std::make_shared(*children_); } -#pragma mark - Equality - -bool ShadowNode::operator==(const ShadowNode& rhs) const { - // Note: Child nodes are not considered as part of instance's value - // and/or identity. - return - tag_ == rhs.tag_ && - rootTag_ == rhs.rootTag_ && - props_ == rhs.props_ && - eventEmitter_ == rhs.eventEmitter_ && - localData_ == rhs.localData_; -} - -bool ShadowNode::operator!=(const ShadowNode& rhs) const { - return !(*this == rhs); -} - #pragma mark - DebugStringConvertible std::string ShadowNode::getDebugName() const { diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.h b/ReactCommon/fabric/core/shadownode/ShadowNode.h index 3642e7d20..610a46550 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.h @@ -101,18 +101,6 @@ public: */ void setLocalData(const SharedLocalData &localData); -#pragma mark - Equality - - /* - * Equality operators. - * Use this to compare `ShadowNode`s values for equality (and non-equality). - * Same values indicates that nodes must not produce mutations - * during tree diffing process. - * Child nodes are not considered as part of the value. - */ - virtual bool operator==(const ShadowNode& rhs) const; - virtual bool operator!=(const ShadowNode& rhs) const; - #pragma mark - DebugStringConvertible std::string getDebugName() const override; diff --git a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp index 386b33460..6429b926a 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp @@ -158,10 +158,10 @@ TEST(ShadowNodeTest, handleLocalData) { thirdNode->setLocalData(localDataOver9000); // LocalData object are compared by pointer, not by value. - ASSERT_EQ(*firstNode, *secondNode); - ASSERT_NE(*firstNode, *thirdNode); + ASSERT_EQ(firstNode->getLocalData(), secondNode->getLocalData()); + ASSERT_NE(firstNode->getLocalData(), thirdNode->getLocalData()); secondNode->setLocalData(anotherLocalData42); - ASSERT_NE(*firstNode, *secondNode); + ASSERT_NE(firstNode->getLocalData(), secondNode->getLocalData()); // LocalData cannot be changed for sealed shadow node. secondNode->sealRecursive(); diff --git a/ReactCommon/fabric/uimanager/Differentiator.cpp b/ReactCommon/fabric/uimanager/Differentiator.cpp index 3e69be60b..6c2c6ea1e 100644 --- a/ReactCommon/fabric/uimanager/Differentiator.cpp +++ b/ReactCommon/fabric/uimanager/Differentiator.cpp @@ -211,12 +211,15 @@ ShadowViewMutationList calculateShadowViewMutations( ShadowViewMutationList mutations; - if (oldRootShadowNode != newRootShadowNode) { + auto oldRootShadowView = ShadowView(oldRootShadowNode); + auto newRootShadowView = ShadowView(newRootShadowNode); + + if (oldRootShadowView != newRootShadowView) { mutations.push_back( ShadowViewMutation::UpdateMutation( ShadowView(), - ShadowView(oldRootShadowNode), - ShadowView(newRootShadowNode), + oldRootShadowView, + newRootShadowView, -1 ) );