From 95074e6c12e0d026cccd8d3c37c2c23b03ea9ae8 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sat, 4 Aug 2018 09:30:20 -0700 Subject: [PATCH] Fabric: ShadowNode::Fragment Summary: @public This diff changes a way how we specify a shape of newly created and/or cloned of ShadowNode. Previously we pass those values as a list of arguments, now those values are coupled into a new data structure called ShadowNodeFragment. All that makes suppose to make code much more easy to read and maintain, this is especially important because we want to add a couple of new entities in this set. Reviewed By: mdvacca Differential Revision: D8988389 fbshipit-source-id: 1835f646e1ecc6a1f413feaf1900f3d3ad0ebc05 --- .../components/view/ConcreteViewShadowNode.h | 43 +++++-------- .../view/accessibility/AccessibleShadowNode.h | 2 +- .../ConcreteComponentDescriptor.h | 24 +++++--- .../fabric/core/shadownode/ShadowNode.cpp | 45 ++++++-------- .../fabric/core/shadownode/ShadowNode.h | 40 ++++++------ .../fabric/core/tests/ShadowNodeTest.cpp | 61 ++++++++++++------- ReactCommon/fabric/uimanager/ShadowTree.cpp | 24 +++++--- 7 files changed, 125 insertions(+), 114 deletions(-) diff --git a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h index e2202e49c6e..da84b55d01b 100644 --- a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h @@ -43,7 +43,6 @@ class ConcreteViewShadowNode: static_assert(std::is_base_of::value, "ViewPropsT must be a descendant of AccessibilityProps"); public: - using ConcreteViewProps = ViewPropsT; using SharedConcreteViewProps = std::shared_ptr; using ConcreteViewEventEmitter = ViewEventEmitterT; @@ -51,55 +50,43 @@ public: using SharedConcreteViewShadowNode = std::shared_ptr; ConcreteViewShadowNode( - const Tag &tag, - const Tag &rootTag, - const SharedConcreteViewProps &props, - const SharedConcreteViewEventEmitter &eventEmitter, - const SharedShadowNodeSharedList &children, + const ShadowNodeFragment &fragment, const ShadowNodeCloneFunction &cloneFunction ): ConcreteShadowNode( - tag, - rootTag, - props, - eventEmitter, - children, + fragment, cloneFunction ), AccessibleShadowNode( - props + std::static_pointer_cast(fragment.props) ), YogaLayoutableShadowNode() { - YogaLayoutableShadowNode::setProps(*props); + YogaLayoutableShadowNode::setProps(*std::static_pointer_cast(fragment.props)); YogaLayoutableShadowNode::setChildren(ConcreteShadowNode::template getChildrenSlice()); }; ConcreteViewShadowNode( - const SharedConcreteViewShadowNode &shadowNode, - const SharedConcreteViewProps &props, - const SharedConcreteViewEventEmitter &eventEmitter, - const SharedShadowNodeSharedList &children + const SharedShadowNode &sourceShadowNode, + const ShadowNodeFragment &fragment ): ConcreteShadowNode( - shadowNode, - props, - eventEmitter, - children + sourceShadowNode, + fragment ), AccessibleShadowNode( - shadowNode, - props + std::static_pointer_cast(sourceShadowNode), + std::static_pointer_cast(fragment.props) ), YogaLayoutableShadowNode( - *shadowNode + *std::static_pointer_cast(sourceShadowNode) ) { - if (props) { - YogaLayoutableShadowNode::setProps(*props); + if (fragment.props) { + YogaLayoutableShadowNode::setProps(*std::static_pointer_cast(fragment.props)); } - if (children) { + if (fragment.children) { YogaLayoutableShadowNode::setChildren(ConcreteShadowNode::template getChildrenSlice()); } }; @@ -119,7 +106,7 @@ public: LayoutableShadowNode *cloneAndReplaceChild(LayoutableShadowNode *child, int suggestedIndex = -1) override { ensureUnsealed(); auto childShadowNode = static_cast(child); - auto clonedChildShadowNode = std::static_pointer_cast(childShadowNode->clone()); + auto clonedChildShadowNode = std::static_pointer_cast(childShadowNode->clone({})); ShadowNode::replaceChild(childShadowNode->shared_from_this(), clonedChildShadowNode, suggestedIndex); return clonedChildShadowNode.get(); } diff --git a/ReactCommon/fabric/components/view/accessibility/AccessibleShadowNode.h b/ReactCommon/fabric/components/view/accessibility/AccessibleShadowNode.h index f893b0dcdca..b5ffab5caea 100644 --- a/ReactCommon/fabric/components/view/accessibility/AccessibleShadowNode.h +++ b/ReactCommon/fabric/components/view/accessibility/AccessibleShadowNode.h @@ -17,7 +17,7 @@ namespace react { class AccessibleShadowNode; -typedef std::shared_ptr SharedAccessibleShadowNode; +using SharedAccessibleShadowNode = std::shared_ptr; class AccessibleShadowNode { diff --git a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h index 2f607fd088c..007dc6a1402 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h @@ -54,11 +54,13 @@ public: assert(std::dynamic_pointer_cast(eventEmitter)); const auto &shadowNode = std::make_shared( - tag, - rootTag, - std::static_pointer_cast(props), - std::static_pointer_cast(eventEmitter), - ShadowNode::emptySharedShadowNodeSharedList(), + ShadowNodeFragment { + .tag = tag, + .rootTag = rootTag, + .props = props, + .eventEmitter = eventEmitter, + .children = ShadowNode::emptySharedShadowNodeSharedList() + }, getCloneFunction() ); @@ -77,9 +79,11 @@ public: const auto &shadowNode = std::make_shared( std::static_pointer_cast(sourceShadowNode), - std::static_pointer_cast(props), - std::static_pointer_cast(eventEmitter), - children + ShadowNodeFragment { + .props = props, + .eventEmitter = eventEmitter, + .children = children + } ); adopt(shadowNode); @@ -124,9 +128,9 @@ private: ShadowNodeCloneFunction getCloneFunction() const { if (!cloneFunction_) { - cloneFunction_ = [this](const SharedShadowNode &shadowNode, const SharedProps &props, const SharedEventEmitter &eventEmitter, const SharedShadowNodeSharedList &children) { + cloneFunction_ = [this](const SharedShadowNode &shadowNode, const ShadowNodeFragment &fragment) { assert(std::dynamic_pointer_cast(shadowNode)); - return this->cloneShadowNode(shadowNode, props, eventEmitter, children); + return this->cloneShadowNode(shadowNode, fragment.props, fragment.eventEmitter, fragment.children); }; } diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.cpp b/ReactCommon/fabric/core/shadownode/ShadowNode.cpp index 91adbc16bda..e67e3d8f62c 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.cpp +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.cpp @@ -23,42 +23,33 @@ SharedShadowNodeSharedList ShadowNode::emptySharedShadowNodeSharedList() { #pragma mark - Constructors ShadowNode::ShadowNode( - const Tag &tag, - const Tag &rootTag, - const SharedProps &props, - const SharedEventEmitter &eventEmitter, - const SharedShadowNodeSharedList &children, + const ShadowNodeFragment &fragment, const ShadowNodeCloneFunction &cloneFunction ): - tag_(tag), - rootTag_(rootTag), - props_(props), - eventEmitter_(eventEmitter), - children_(std::make_shared(*children)), + tag_(fragment.tag), + rootTag_(fragment.rootTag), + props_(fragment.props), + eventEmitter_(fragment.eventEmitter), + children_(std::make_shared(*fragment.children)), cloneFunction_(cloneFunction), revision_(1) {} ShadowNode::ShadowNode( - const SharedShadowNode &shadowNode, - const SharedProps &props, - const SharedEventEmitter &eventEmitter, - const SharedShadowNodeSharedList &children + const SharedShadowNode &sourceShadowNode, + const ShadowNodeFragment &fragment ): - tag_(shadowNode->tag_), - rootTag_(shadowNode->rootTag_), - props_(props ? props : shadowNode->props_), - eventEmitter_(eventEmitter ? eventEmitter : shadowNode->eventEmitter_), - children_(std::make_shared(*(children ? children : shadowNode->children_))), - localData_(shadowNode->localData_), - cloneFunction_(shadowNode->cloneFunction_), - revision_(shadowNode->revision_ + 1) {} + tag_(fragment.tag ?: sourceShadowNode->tag_), + rootTag_(fragment.rootTag ?: sourceShadowNode->rootTag_), + props_(fragment.props ?: sourceShadowNode->props_), + eventEmitter_(fragment.eventEmitter ?: sourceShadowNode->eventEmitter_), + children_(std::make_shared(*(fragment.children ?: sourceShadowNode->children_))), + localData_(fragment.localData ?: sourceShadowNode->localData_), + cloneFunction_(sourceShadowNode->cloneFunction_), + revision_(sourceShadowNode->revision_ + 1) {} -UnsharedShadowNode ShadowNode::clone( - const SharedProps &props, - const SharedShadowNodeSharedList &children -) const { +UnsharedShadowNode ShadowNode::clone(const ShadowNodeFragment &fragment) const { assert(cloneFunction_); - return cloneFunction_(shared_from_this(), props_, eventEmitter_, children_); + return cloneFunction_(shared_from_this(), fragment); } #pragma mark - Getters diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.h b/ReactCommon/fabric/core/shadownode/ShadowNode.h index 173b171f8d8..3043d1e630a 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.h @@ -29,45 +29,51 @@ using SharedShadowNodeList = std::vector>; using SharedShadowNodeSharedList = std::shared_ptr; using SharedShadowNodeUnsharedList = std::shared_ptr; +struct ShadowNodeFragment { + Tag tag; + Tag rootTag; + SharedProps props; + SharedEventEmitter eventEmitter; + SharedShadowNodeSharedList children; + SharedLocalData localData; +}; + using ShadowNodeCloneFunction = std::function; class ShadowNode: public virtual Sealable, public virtual DebugStringConvertible, public std::enable_shared_from_this { + public: static SharedShadowNodeSharedList emptySharedShadowNodeSharedList(); #pragma mark - Constructors + /* + * Creates a Shadow Node based on fields specified in a `fragment`. + */ ShadowNode( - const Tag &tag, - const Tag &rootTag, - const SharedProps &props, - const SharedEventEmitter &eventEmitter, - const SharedShadowNodeSharedList &children, + const ShadowNodeFragment &fragment, const ShadowNodeCloneFunction &cloneFunction ); + /* + * Creates a Shadow Node via cloning given `sourceShadowNode` and + * applying fields from given `fragment`. + */ ShadowNode( - const SharedShadowNode &shadowNode, - const SharedProps &props, - const SharedEventEmitter &eventEmitter, - const SharedShadowNodeSharedList &children + const SharedShadowNode &sourceShadowNode, + const ShadowNodeFragment &fragment ); /* * Clones the shadow node using stored `cloneFunction`. */ - UnsharedShadowNode clone( - const SharedProps &props = nullptr, - const SharedShadowNodeSharedList &children = nullptr - ) const; + UnsharedShadowNode clone(const ShadowNodeFragment &fragment) const; #pragma mark - Getters diff --git a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp index aa058375db7..cb64952883f 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp @@ -28,7 +28,15 @@ TEST(ShadowNodeTest, handleProps) { } TEST(ShadowNodeTest, handleShadowNodeCreation) { - auto node = std::make_shared(9, 1, std::make_shared(), nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto node = std::make_shared( + ShadowNodeFragment { + .tag = 9, + .rootTag = 1, + .props = std::make_shared(), + .children = ShadowNode::emptySharedShadowNodeSharedList() + }, + nullptr + ); ASSERT_FALSE(node->getSealed()); ASSERT_STREQ(node->getComponentName().c_str(), "Test"); @@ -45,8 +53,16 @@ TEST(ShadowNodeTest, handleShadowNodeCreation) { } TEST(ShadowNodeTest, handleShadowNodeSimpleCloning) { - auto node = std::make_shared(9, 1, std::make_shared(), nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); - auto node2 = std::make_shared(node, nullptr, nullptr, nullptr); + auto node = std::make_shared( + ShadowNodeFragment { + .tag = 9, + .rootTag = 1, + .props = std::make_shared(), + .children = ShadowNode::emptySharedShadowNodeSharedList() + }, + nullptr + ); + auto node2 = std::make_shared(node, ShadowNodeFragment {}); ASSERT_STREQ(node->getComponentName().c_str(), "Test"); ASSERT_EQ(node->getTag(), 9); @@ -56,9 +72,9 @@ TEST(ShadowNodeTest, handleShadowNodeSimpleCloning) { TEST(ShadowNodeTest, handleShadowNodeMutation) { auto props = std::make_shared(); - auto node1 = std::make_shared(1, 1, props, nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); - auto node2 = std::make_shared(2, 1, props, nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); - auto node3 = std::make_shared(3, 1, props, nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto node1 = std::make_shared(ShadowNodeFragment {.tag = 1, .rootTag = 1, .props = std::make_shared(), .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); + auto node2 = std::make_shared(ShadowNodeFragment {.tag = 2, .rootTag = 1, .props = std::make_shared(), .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); + auto node3 = std::make_shared(ShadowNodeFragment {.tag = 3, .rootTag = 1, .props = std::make_shared(), .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); node1->appendChild(node2); node1->appendChild(node3); @@ -67,7 +83,7 @@ TEST(ShadowNodeTest, handleShadowNodeMutation) { ASSERT_EQ(node1Children->at(0), node2); ASSERT_EQ(node1Children->at(1), node3); - auto node4 = std::make_shared(node2, nullptr, nullptr, nullptr); + auto node4 = std::make_shared(node2, ShadowNodeFragment {}); node1->replaceChild(node2, node4); node1Children = node1->getChildren(); ASSERT_EQ(node1Children->size(), 2); @@ -83,34 +99,33 @@ TEST(ShadowNodeTest, handleShadowNodeMutation) { // No more mutation after sealing. EXPECT_THROW(node4->setLocalData(nullptr), std::runtime_error); - auto node5 = std::make_shared(node4, nullptr, nullptr, nullptr); + auto node5 = std::make_shared(node4, ShadowNodeFragment {}); node5->setLocalData(nullptr); ASSERT_EQ(node5->getLocalData(), nullptr); } TEST(ShadowNodeTest, handleCloneFunction) { - auto firstNode = std::make_shared(9, 1, std::make_shared(), nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto firstNode = std::make_shared(ShadowNodeFragment {.tag = 9, .rootTag = 1, .props = std::make_shared(), .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); // The shadow node is not clonable if `cloneFunction` is not provided, - ASSERT_DEATH_IF_SUPPORTED(firstNode->clone(), "cloneFunction_"); + ASSERT_DEATH_IF_SUPPORTED(firstNode->clone({}), "cloneFunction_"); auto secondNode = std::make_shared( - 9, - 1, - std::make_shared(), - nullptr, - ShadowNode::emptySharedShadowNodeSharedList(), - [](const SharedShadowNode &shadowNode, const SharedProps &props, const SharedEventEmitter &eventEmitter, const SharedShadowNodeSharedList &children) { + ShadowNodeFragment { + .tag = 9, + .rootTag = 1, + .props = std::make_shared(), + .children = ShadowNode::emptySharedShadowNodeSharedList() + }, + [](const SharedShadowNode &shadowNode, const ShadowNodeFragment &fragment) { return std::make_shared( std::static_pointer_cast(shadowNode), - props, - nullptr, - children + fragment ); } ); - auto secondNodeClone = secondNode->clone(); + auto secondNodeClone = secondNode->clone({}); // Those two nodes are *not* same. ASSERT_NE(secondNode, secondNodeClone); @@ -134,9 +149,9 @@ TEST(ShadowNodeTest, handleLocalData) { auto localDataOver9000 = std::make_shared(); localDataOver9000->setNumber(9001); auto props = std::make_shared(); - auto firstNode = std::make_shared(9, 1, props, nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); - auto secondNode = std::make_shared(9, 1, props, nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); - auto thirdNode = std::make_shared(9, 1, props, nullptr, ShadowNode::emptySharedShadowNodeSharedList(), nullptr); + auto firstNode = std::make_shared(ShadowNodeFragment {.tag = 9, .rootTag = 1, .props = props, .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); + auto secondNode = std::make_shared(ShadowNodeFragment {.tag = 9, .rootTag = 1, .props = props, .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); + auto thirdNode = std::make_shared(ShadowNodeFragment {.tag = 9, .rootTag = 1, .props = props, .children = ShadowNode::emptySharedShadowNodeSharedList()}, nullptr); firstNode->setLocalData(localData42); secondNode->setLocalData(localData42); diff --git a/ReactCommon/fabric/uimanager/ShadowTree.cpp b/ReactCommon/fabric/uimanager/ShadowTree.cpp index d179d9dcad1..e52da656a2b 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.cpp +++ b/ReactCommon/fabric/uimanager/ShadowTree.cpp @@ -18,13 +18,15 @@ namespace react { ShadowTree::ShadowTree(Tag rootTag): rootTag_(rootTag) { - const auto &noopEventEmitter = std::make_shared(nullptr, rootTag, nullptr); + const auto noopEventEmitter = std::make_shared(nullptr, rootTag, nullptr); rootShadowNode_ = std::make_shared( - rootTag, - rootTag, - RootShadowNode::defaultSharedProps(), - noopEventEmitter, - ShadowNode::emptySharedShadowNodeSharedList(), + ShadowNodeFragment { + .tag = rootTag, + .rootTag = rootTag, + .props = RootShadowNode::defaultSharedProps(), + .eventEmitter = noopEventEmitter, + .children = ShadowNode::emptySharedShadowNodeSharedList(), + }, nullptr ); } @@ -51,14 +53,20 @@ void ShadowTree::constraintLayout(const LayoutConstraints &layoutConstraints, co UnsharedRootShadowNode ShadowTree::cloneRootShadowNode(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const { auto oldRootShadowNode = rootShadowNode_; const auto &props = std::make_shared(*oldRootShadowNode->getProps(), layoutConstraints, layoutContext); - auto newRootShadowNode = std::make_shared(oldRootShadowNode, props, nullptr, nullptr); + auto newRootShadowNode = + std::make_shared(oldRootShadowNode, ShadowNodeFragment {.props = props}); return newRootShadowNode; } void ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) { auto oldRootShadowNode = rootShadowNode_; auto newRootShadowNode = - std::make_shared(oldRootShadowNode, nullptr, nullptr, SharedShadowNodeSharedList(rootChildNodes)); + std::make_shared( + oldRootShadowNode, + ShadowNodeFragment { + .children = SharedShadowNodeSharedList(rootChildNodes) + } + ); complete(newRootShadowNode); }