From 8f507280acfa09340364c24bd5f445464b0d1b86 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Thu, 17 May 2018 20:03:42 -0700 Subject: [PATCH] Fabric: Removing default arguments from ShadowNode's constructors Summary: We don't use them at all; moreover they complicate adding/changing signatures of those methods (because arguments with defaults must be grouped at the end and some arguments cannot have defaults). Reviewed By: fkgozali Differential Revision: D7981456 fbshipit-source-id: d7dd098e83630d1ab3342d2ca52ade9c4e27b2c3 --- .../core/shadownode/ConcreteShadowNode.h | 11 +++--- .../fabric/core/shadownode/ShadowNode.h | 10 +++--- .../fabric/core/tests/ShadowNodeTest.cpp | 35 ++++++++++--------- ReactCommon/fabric/uimanager/ShadowTree.cpp | 4 ++- .../fabric/view/ConcreteViewShadowNode.h | 10 +++--- 5 files changed, 36 insertions(+), 34 deletions(-) diff --git a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h index 93663a325..6cf911ea1 100644 --- a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h @@ -41,9 +41,9 @@ public: const Tag &tag, const Tag &rootTag, const InstanceHandle &instanceHandle, - const SharedConcreteProps &props = ConcreteShadowNode::defaultSharedProps(), - const SharedShadowNodeSharedList &children = ShadowNode::emptySharedShadowNodeSharedList(), - const ShadowNodeCloneFunction &cloneFunction = nullptr + const SharedConcreteProps &props, + const SharedShadowNodeSharedList &children, + const ShadowNodeCloneFunction &cloneFunction ): ShadowNode( tag, @@ -56,8 +56,8 @@ public: ConcreteShadowNode( const SharedConcreteShadowNode &shadowNode, - const SharedProps &props = nullptr, - const SharedShadowNodeSharedList &children = nullptr + const SharedProps &props, + const SharedShadowNodeSharedList &children ): ShadowNode( shadowNode, @@ -73,7 +73,6 @@ public: assert(std::dynamic_pointer_cast(props_)); return std::static_pointer_cast(props_); } - }; } // namespace react diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.h b/ReactCommon/fabric/core/shadownode/ShadowNode.h index fec74d3ab..927590b86 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.h @@ -44,15 +44,15 @@ public: const Tag &tag, const Tag &rootTag, const InstanceHandle &instanceHandle, - const SharedProps &props = SharedProps(), - const SharedShadowNodeSharedList &children = SharedShadowNodeSharedList(), - const ShadowNodeCloneFunction &cloneFunction = nullptr + const SharedProps &props, + const SharedShadowNodeSharedList &children, + const ShadowNodeCloneFunction &cloneFunction ); ShadowNode( const SharedShadowNode &shadowNode, - const SharedProps &props = nullptr, - const SharedShadowNodeSharedList &children = nullptr + const SharedProps &props, + const SharedShadowNodeSharedList &children ); /* diff --git a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp index 3395763d9..dfdf0dec4 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp @@ -28,7 +28,7 @@ TEST(ShadowNodeTest, handleProps) { } TEST(ShadowNodeTest, handleShadowNodeCreation) { - auto node = std::make_shared(9, 1, (void *)NULL); + auto node = std::make_shared(9, 1, (void *)NULL, std::make_shared(), ShadowNode::emptySharedShadowNodeSharedList(), nullptr); ASSERT_FALSE(node->getSealed()); ASSERT_STREQ(node->getComponentName().c_str(), "Test"); @@ -48,8 +48,8 @@ TEST(ShadowNodeTest, handleShadowNodeCreation) { } TEST(ShadowNodeTest, handleShadowNodeSimpleCloning) { - auto node = std::make_shared(9, 1, (void *)NULL); - auto node2 = std::make_shared(node); + auto node = std::make_shared(9, 1, (void *)NULL, std::make_shared(), ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto node2 = std::make_shared(node, nullptr, nullptr); ASSERT_STREQ(node->getComponentName().c_str(), "Test"); ASSERT_EQ(node->getTag(), 9); @@ -59,9 +59,10 @@ TEST(ShadowNodeTest, handleShadowNodeSimpleCloning) { } TEST(ShadowNodeTest, handleShadowNodeMutation) { - auto node1 = std::make_shared(1, 1, (void *)NULL); - auto node2 = std::make_shared(2, 1, (void *)NULL); - auto node3 = std::make_shared(3, 1, (void *)NULL); + auto props = std::make_shared(); + auto node1 = std::make_shared(1, 1, (void *)NULL, props, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto node2 = std::make_shared(2, 1, (void *)NULL, props, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto node3 = std::make_shared(3, 1, (void *)NULL, props, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); node1->appendChild(node2); node1->appendChild(node3); @@ -70,7 +71,7 @@ TEST(ShadowNodeTest, handleShadowNodeMutation) { ASSERT_EQ(node1Children->at(0), node2); ASSERT_EQ(node1Children->at(1), node3); - auto node4 = std::make_shared(node2); + auto node4 = std::make_shared(node2, nullptr, nullptr); node1->replaceChild(node2, node4); node1Children = node1->getChildren(); ASSERT_EQ(node1Children->size(), 2); @@ -86,16 +87,16 @@ TEST(ShadowNodeTest, handleShadowNodeMutation) { // No more mutation after sealing. EXPECT_THROW(node4->clearSourceNode(), std::runtime_error); - auto node5 = std::make_shared(node4); + auto node5 = std::make_shared(node4, nullptr, nullptr); node5->clearSourceNode(); ASSERT_EQ(node5->getSourceNode(), nullptr); } TEST(ShadowNodeTest, handleSourceNode) { - auto nodeFirstGeneration = std::make_shared(9, 1, (void *)NULL); - auto nodeSecondGeneration = std::make_shared(nodeFirstGeneration); - auto nodeThirdGeneration = std::make_shared(nodeSecondGeneration); - auto nodeForthGeneration = std::make_shared(nodeThirdGeneration); + auto nodeFirstGeneration = std::make_shared(9, 1, (void *)NULL, std::make_shared(), ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto nodeSecondGeneration = std::make_shared(nodeFirstGeneration, nullptr, nullptr); + auto nodeThirdGeneration = std::make_shared(nodeSecondGeneration, nullptr, nullptr); + auto nodeForthGeneration = std::make_shared(nodeThirdGeneration, nullptr, nullptr); // Ensure established shource nodes structure. ASSERT_EQ(nodeForthGeneration->getSourceNode(), nodeThirdGeneration); @@ -116,7 +117,7 @@ TEST(ShadowNodeTest, handleSourceNode) { } TEST(ShadowNodeTest, handleCloneFunction) { - auto firstNode = std::make_shared(9, 1, (void *)NULL); + auto firstNode = std::make_shared(9, 1, (void *)NULL, std::make_shared(), ShadowNode::emptySharedShadowNodeSharedList(), nullptr); // The shadow node is not clonable if `cloneFunction` is not provided, ASSERT_DEATH_IF_SUPPORTED(firstNode->clone(), "cloneFunction_"); @@ -159,10 +160,10 @@ TEST(ShadowNodeTest, handleLocalData) { auto localDataOver9000 = std::make_shared(); localDataOver9000->setNumber(9001); - - auto firstNode = std::make_shared(9, 1, (void *)NULL); - auto secondNode = std::make_shared(9, 1, (void *)NULL); - auto thirdNode = std::make_shared(9, 1, (void *)NULL); + auto props = std::make_shared(); + auto firstNode = std::make_shared(9, 1, (void *)NULL, props, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto secondNode = std::make_shared(9, 1, (void *)NULL, props, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto thirdNode = std::make_shared(9, 1, (void *)NULL, props, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); firstNode->setLocalData(localData42); secondNode->setLocalData(localData42); diff --git a/ReactCommon/fabric/uimanager/ShadowTree.cpp b/ReactCommon/fabric/uimanager/ShadowTree.cpp index 30b246438..c274d2e68 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.cpp +++ b/ReactCommon/fabric/uimanager/ShadowTree.cpp @@ -19,7 +19,9 @@ ShadowTree::ShadowTree(Tag rootTag): rootTag, rootTag, nullptr, - RootShadowNode::defaultSharedProps() + RootShadowNode::defaultSharedProps(), + ShadowNode::emptySharedShadowNodeSharedList(), + nullptr ); } diff --git a/ReactCommon/fabric/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/view/ConcreteViewShadowNode.h index 345e3fc46..83149be33 100644 --- a/ReactCommon/fabric/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/view/ConcreteViewShadowNode.h @@ -43,9 +43,9 @@ public: const Tag &tag, const Tag &rootTag, const InstanceHandle &instanceHandle, - const SharedConcreteViewProps &props = ConcreteViewShadowNode::defaultSharedProps(), - const SharedShadowNodeSharedList &children = ShadowNode::emptySharedShadowNodeSharedList(), - const ShadowNodeCloneFunction &cloneFunction = nullptr + const SharedConcreteViewProps &props, + const SharedShadowNodeSharedList &children, + const ShadowNodeCloneFunction &cloneFunction ): ConcreteShadowNode( tag, @@ -65,8 +65,8 @@ public: ConcreteViewShadowNode( const SharedConcreteViewShadowNode &shadowNode, - const SharedConcreteViewProps &props = nullptr, - const SharedShadowNodeSharedList &children = nullptr + const SharedConcreteViewProps &props, + const SharedShadowNodeSharedList &children ): ConcreteShadowNode( shadowNode,