From 3fd2e2da4fd5881e469cfda1f2e261e880fb1cab Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 16 Apr 2018 07:43:22 -0700 Subject: [PATCH] Fabric/Text/Prep: Refined ComponentDescriptor interface Summary: The new interface of ComponentDescriptor makes ShadowNode creation/cloning process a bit more explicit: Now customers (UIManager) must prepare Props object explicitly before creation or cloning. Besides general clarity, we need this to prepare for a new virtual `ShadowNode::clone()` method which will serve "virtual constructor" role, redirecting execution to concrete ComponentDescriptor instance. Actually, the whole purpose of concrete ComponentDescriptor instance is serve "virtual constructor" role (and all this code should be "templated"). Reviewed By: mdvacca Differential Revision: D7591714 fbshipit-source-id: 8793b3ef70ed7ae113efb36ad1eee20573360dc8 --- .../componentdescriptor/ComponentDescriptor.h | 15 ++++++- .../ConcreteComponentDescriptor.h | 18 +++++--- .../core/tests/ComponentDescriptorTest.cpp | 13 +++--- .../fabric/uimanager/FabricUIManager.cpp | 42 ++++++++++++++++--- 4 files changed, 70 insertions(+), 18 deletions(-) diff --git a/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h index 7b7b7a557..1b92620d9 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h @@ -48,7 +48,7 @@ public: const Tag &tag, const Tag &rootTag, const InstanceHandle &instanceHandle, - const RawProps &rawProps + const SharedProps &props ) const = 0; /* @@ -56,7 +56,7 @@ public: */ virtual SharedShadowNode cloneShadowNode( const SharedShadowNode &shadowNode, - const SharedRawProps &rawProps = nullptr, + const SharedProps &props = nullptr, const SharedShadowNodeSharedList &children = nullptr ) const = 0; @@ -67,6 +67,17 @@ public: const SharedShadowNode &parentShadowNode, const SharedShadowNode &childShadowNode ) const = 0; + + /* + * Creates a new `Props` of a particular type with all values copied from + * `props` and `rawProps` applied on top of this. + * If `props` is `nullptr`, a default `Props` object (with default values) + * will be used. + */ + virtual SharedProps cloneProps( + const SharedProps &props, + const RawProps &rawProps + ) const = 0; }; } // namespace react diff --git a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h index 95de5e175..8739d849a 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h @@ -26,6 +26,7 @@ class ConcreteComponentDescriptor: public ComponentDescriptor { static_assert(std::is_base_of::value, "ShadowNodeT must be a descendant of ShadowNode"); using SharedShadowNodeT = std::shared_ptr; + using ConcreteProps = typename ShadowNodeT::ConcreteProps; using SharedConcreteProps = typename ShadowNodeT::SharedConcreteProps; public: @@ -37,19 +38,17 @@ public: const Tag &tag, const Tag &rootTag, const InstanceHandle &instanceHandle, - const RawProps &rawProps + const SharedProps &props ) const override { - auto props = ShadowNodeT::Props(rawProps); - return std::make_shared(tag, rootTag, instanceHandle, props); + return std::make_shared(tag, rootTag, instanceHandle, std::static_pointer_cast(props)); } SharedShadowNode cloneShadowNode( const SharedShadowNode &shadowNode, - const SharedRawProps &rawProps = nullptr, + const SharedProps &props = nullptr, const SharedShadowNodeSharedList &children = nullptr ) const override { - const SharedConcreteProps props = rawProps ? ShadowNodeT::Props(*rawProps, shadowNode->getProps()) : nullptr; - return std::make_shared(std::static_pointer_cast(shadowNode), props, children); + return std::make_shared(std::static_pointer_cast(shadowNode), std::static_pointer_cast(props), children); } void appendChild( @@ -61,6 +60,13 @@ public: concreteNonConstParentShadowNode->appendChild(childShadowNode); } + virtual SharedProps cloneProps( + const SharedProps &props, + const RawProps &rawProps + ) const override { + return ShadowNodeT::Props(rawProps, props); + }; + }; } // namespace react diff --git a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp index 699642ce7..5c2a0bc9d 100644 --- a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp +++ b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp @@ -19,7 +19,8 @@ TEST(ComponentDescriptorTest, createShadowNode) { RawProps raw; raw["nativeID"] = "abc"; - SharedShadowNode node = descriptor->createShadowNode(9, 1, (void *)NULL, raw); + SharedProps props = descriptor->cloneProps(nullptr, raw); + SharedShadowNode node = descriptor->createShadowNode(9, 1, (void *)NULL, props); ASSERT_EQ(node->getComponentHandle(), typeid(TestShadowNode).hash_code()); ASSERT_STREQ(node->getComponentName().c_str(), "Test"); @@ -35,7 +36,8 @@ TEST(ComponentDescriptorTest, cloneShadowNode) { RawProps raw; raw["nativeID"] = "abc"; - SharedShadowNode node = descriptor->createShadowNode(9, 1, (void *)NULL, raw); + SharedProps props = descriptor->cloneProps(nullptr, raw); + SharedShadowNode node = descriptor->createShadowNode(9, 1, (void *)NULL, props); SharedShadowNode cloned = descriptor->cloneShadowNode(node); ASSERT_EQ(cloned->getComponentHandle(), typeid(TestShadowNode).hash_code()); @@ -52,9 +54,10 @@ TEST(ComponentDescriptorTest, appendChild) { RawProps raw; raw["nativeID"] = "abc"; - SharedShadowNode node1 = descriptor->createShadowNode(1, 1, (void *)NULL, raw); - SharedShadowNode node2 = descriptor->createShadowNode(2, 1, (void *)NULL, raw); - SharedShadowNode node3 = descriptor->createShadowNode(3, 1, (void *)NULL, raw); + SharedProps props = descriptor->cloneProps(nullptr, raw); + SharedShadowNode node1 = descriptor->createShadowNode(1, 1, (void *)NULL, props); + SharedShadowNode node2 = descriptor->createShadowNode(2, 1, (void *)NULL, props); + SharedShadowNode node3 = descriptor->createShadowNode(3, 1, (void *)NULL, props); descriptor->appendChild(node1, node2); descriptor->appendChild(node1, node3); diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.cpp b/ReactCommon/fabric/uimanager/FabricUIManager.cpp index 5ba9138d9..202073283 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.cpp +++ b/ReactCommon/fabric/uimanager/FabricUIManager.cpp @@ -54,7 +54,15 @@ SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int LOG(INFO) << "FabricUIManager::createNode(tag: " << tag << ", name: " << viewName << ", rootTag" << rootTag << ", props: " << props << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)["View"]; RawProps rawProps = rawPropsFromDynamic(props); - SharedShadowNode shadowNode = componentDescriptor->createShadowNode(tag, rootTag, instanceHandle, rawProps); + + SharedShadowNode shadowNode = + componentDescriptor->createShadowNode( + tag, + rootTag, + instanceHandle, + componentDescriptor->cloneProps(nullptr, rawProps) + ); + LOG(INFO) << "FabricUIManager::createNode() -> " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}); if (delegate_) { @@ -67,7 +75,10 @@ SharedShadowNode FabricUIManager::createNode(int tag, std::string viewName, int SharedShadowNode FabricUIManager::cloneNode(const SharedShadowNode &shadowNode) { LOG(INFO) << "FabricUIManager::cloneNode(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; - SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode(shadowNode); + + SharedShadowNode clonedShadowNode = + componentDescriptor->cloneShadowNode(shadowNode); + LOG(INFO) << "FabricUIManager::cloneNode() -> " << clonedShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}); return clonedShadowNode; } @@ -76,7 +87,14 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildren(const SharedShadowNod LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildren(shadowNode: " << shadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; // Assuming semantic: Cloning with same props but empty children. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; - SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode(shadowNode, nullptr, ShadowNode::emptySharedShadowNodeSharedList()); + + SharedShadowNode clonedShadowNode = + componentDescriptor->cloneShadowNode( + shadowNode, + nullptr, + ShadowNode::emptySharedShadowNodeSharedList() + ); + LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildren() -> " << clonedShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}); return clonedShadowNode; } @@ -86,7 +104,14 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewProps(const SharedShadowNode & // Assuming semantic: Cloning with same children and specified props. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; RawProps rawProps = rawPropsFromDynamic(props); - SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode(shadowNode, std::make_shared(rawProps), nullptr); + + SharedShadowNode clonedShadowNode = + componentDescriptor->cloneShadowNode( + shadowNode, + componentDescriptor->cloneProps(shadowNode->getProps(), rawProps), + nullptr + ); + LOG(INFO) << "FabricUIManager::cloneNodeWithNewProps() -> " << clonedShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}); return clonedShadowNode; } @@ -96,7 +121,14 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedS // Assuming semantic: Cloning with empty children and specified props. const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[shadowNode]; RawProps rawProps = rawPropsFromDynamic(props); - SharedShadowNode clonedShadowNode = componentDescriptor->cloneShadowNode(shadowNode, std::make_shared(rawProps), ShadowNode::emptySharedShadowNodeSharedList()); + + SharedShadowNode clonedShadowNode = + componentDescriptor->cloneShadowNode( + shadowNode, + componentDescriptor->cloneProps(shadowNode->getProps(), rawProps), + ShadowNode::emptySharedShadowNodeSharedList() + ); + LOG(INFO) << "FabricUIManager::cloneNodeWithNewChildrenAndProps() -> " << clonedShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}); return clonedShadowNode; }