From 67a79010cab155d2e44c8a253a9763355aa1e10c Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sat, 4 Aug 2018 09:30:13 -0700 Subject: [PATCH] Fabric: Simplified way to specialize ComponentName in ConcreteShadowNode class template Summary: @public Previously, all ConcreteShadowNode subclasses had to override `getComponentName()` function to specialize a name of the component. And often it was all that those subclasses do. Now, it's a template argument; and many ShadowNode classes can be created as oneliners via *just* specializing ConcreteShadowNode template. Unfortunately, C++ does not allow to use `std::string`s or string literals as template arguments, but it allows to use pointers. Moreover, those pointers must point to some linked data, hence, those values must be declared in .cpp (not .h) files. For simplicity, we put those constants in Props classes, (but this is not a strong requirement). Reviewed By: mdvacca Differential Revision: D8942826 fbshipit-source-id: 4fd517e2485eb8f8c20a51df9b3496941856d8a5 --- .../ActivityIndicatorViewShadowNode.cpp | 6 ++--- .../ActivityIndicatorViewShadowNode.h | 16 ++++++-------- .../components/image/ImageShadowNode.cpp | 4 +--- .../fabric/components/image/ImageShadowNode.h | 10 ++++++--- .../fabric/components/root/RootProps.h | 1 - .../fabric/components/root/RootShadowNode.cpp | 4 +--- .../fabric/components/root/RootShadowNode.h | 9 +++++--- .../scrollview/ScrollViewShadowNode.cpp | 4 +--- .../scrollview/ScrollViewShadowNode.h | 14 +++++------- .../components/switch/SwitchShadowNode.cpp | 6 ++--- .../components/switch/SwitchShadowNode.h | 17 +++++++------- .../text/basetext/BaseTextShadowNode.cpp | 4 ++-- .../paragraph/ParagraphComponentDescriptor.h | 2 +- .../text/paragraph/ParagraphProps.h | 4 ---- .../text/paragraph/ParagraphShadowNode.cpp | 10 +++------ .../text/paragraph/ParagraphShadowNode.h | 11 +++++----- .../components/text/rawtext/RawTextProps.h | 1 - .../text/rawtext/RawTextShadowNode.cpp | 6 +---- .../text/rawtext/RawTextShadowNode.h | 20 +++++------------ .../fabric/components/text/text/TextProps.h | 5 ----- .../components/text/text/TextShadowNode.cpp | 6 +---- .../components/text/text/TextShadowNode.h | 15 +++++-------- .../components/view/ConcreteViewShadowNode.h | 20 ++++++++++++----- .../fabric/components/view/ViewProps.h | 1 - .../fabric/components/view/ViewShadowNode.cpp | 4 +--- .../fabric/components/view/ViewShadowNode.h | 21 ++++++------------ .../ConcreteComponentDescriptor.h | 17 ++++---------- .../fabric/core/primitives/ReactPrimitives.h | 2 +- .../core/shadownode/ConcreteShadowNode.h | 22 ++++++++++++++++--- .../core/tests/ComponentDescriptorTest.cpp | 7 +++--- .../fabric/core/tests/ShadowNodeTest.cpp | 2 -- ReactCommon/fabric/core/tests/TestComponent.h | 13 +++-------- 32 files changed, 118 insertions(+), 166 deletions(-) diff --git a/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.cpp b/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.cpp index a19b2f717..d32f545ba 100644 --- a/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.cpp +++ b/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.cpp @@ -5,14 +5,12 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include "ActivityIndicatorViewShadowNode.h" namespace facebook { namespace react { -ComponentName ActivityIndicatorViewShadowNode::getComponentName() const { - return ComponentName("ActivityIndicatorView"); -} +const char ActivityIndicatorViewComponentName[] = "ActivityIndicatorView"; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.h b/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.h index c93462d84..ab24e53a0 100644 --- a/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.h +++ b/ReactCommon/fabric/components/activityindicator/ActivityIndicatorViewShadowNode.h @@ -13,18 +13,16 @@ namespace facebook { namespace react { +extern const char ActivityIndicatorViewComponentName[]; + /* * `ShadowNode` for component. */ -class ActivityIndicatorViewShadowNode final: - public ConcreteViewShadowNode { - -public: - - using ConcreteViewShadowNode::ConcreteViewShadowNode; - - ComponentName getComponentName() const override; -}; +using ActivityIndicatorViewShadowNode = + ConcreteViewShadowNode< + ActivityIndicatorViewComponentName, + ActivityIndicatorViewProps + >; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/image/ImageShadowNode.cpp b/ReactCommon/fabric/components/image/ImageShadowNode.cpp index c9849fb34..37353b474 100644 --- a/ReactCommon/fabric/components/image/ImageShadowNode.cpp +++ b/ReactCommon/fabric/components/image/ImageShadowNode.cpp @@ -14,9 +14,7 @@ namespace facebook { namespace react { -ComponentName ImageShadowNode::getComponentName() const { - return ComponentName("Image"); -} +const char ImageComponentName[] = "Image"; void ImageShadowNode::setImageManager(const SharedImageManager &imageManager) { ensureUnsealed(); diff --git a/ReactCommon/fabric/components/image/ImageShadowNode.h b/ReactCommon/fabric/components/image/ImageShadowNode.h index f8148c94d..c818559fb 100644 --- a/ReactCommon/fabric/components/image/ImageShadowNode.h +++ b/ReactCommon/fabric/components/image/ImageShadowNode.h @@ -16,18 +16,22 @@ namespace facebook { namespace react { +extern const char ImageComponentName[]; + /* * `ShadowNode` for component. */ class ImageShadowNode final: - public ConcreteViewShadowNode { + public ConcreteViewShadowNode< + ImageComponentName, + ImageProps, + ImageEventEmitter + > { public: using ConcreteViewShadowNode::ConcreteViewShadowNode; - ComponentName getComponentName() const override; - /* * Associates a shared `ImageManager` with the node. */ diff --git a/ReactCommon/fabric/components/root/RootProps.h b/ReactCommon/fabric/components/root/RootProps.h index b6c4a2ae5..1804454fe 100644 --- a/ReactCommon/fabric/components/root/RootProps.h +++ b/ReactCommon/fabric/components/root/RootProps.h @@ -24,7 +24,6 @@ class RootProps final: public ViewProps { public: - RootProps() = default; RootProps( const RootProps &sourceProps, diff --git a/ReactCommon/fabric/components/root/RootShadowNode.cpp b/ReactCommon/fabric/components/root/RootShadowNode.cpp index 045be4569..f7787a840 100644 --- a/ReactCommon/fabric/components/root/RootShadowNode.cpp +++ b/ReactCommon/fabric/components/root/RootShadowNode.cpp @@ -12,9 +12,7 @@ namespace facebook { namespace react { -ComponentName RootShadowNode::getComponentName() const { - return ComponentName("RootView"); -} +const char RootComponentName[] = "RootView"; void RootShadowNode::layout() { ensureUnsealed(); diff --git a/ReactCommon/fabric/components/root/RootShadowNode.h b/ReactCommon/fabric/components/root/RootShadowNode.h index b6d96db38..113cabe5c 100644 --- a/ReactCommon/fabric/components/root/RootShadowNode.h +++ b/ReactCommon/fabric/components/root/RootShadowNode.h @@ -21,6 +21,8 @@ class RootShadowNode; using SharedRootShadowNode = std::shared_ptr; using UnsharedRootShadowNode = std::shared_ptr; +extern const char RootComponentName[]; + /* * `ShadowNode` for the root component. * Besides all functionality of the `View` component, `RootShadowNode` contains @@ -28,14 +30,15 @@ using UnsharedRootShadowNode = std::shared_ptr; * shadow tree. */ class RootShadowNode final: - public ConcreteViewShadowNode { + public ConcreteViewShadowNode< + RootComponentName, + RootProps + > { public: using ConcreteViewShadowNode::ConcreteViewShadowNode; - ComponentName getComponentName() const override; - /* * Layouts the shadow tree. */ diff --git a/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.cpp b/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.cpp index 5662889f2..70296a183 100644 --- a/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.cpp +++ b/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.cpp @@ -14,9 +14,7 @@ namespace facebook { namespace react { -ComponentName ScrollViewShadowNode::getComponentName() const { - return ComponentName("ScrollView"); -} +const char ScrollViewComponentName[] = "ScrollView"; void ScrollViewShadowNode::updateLocalData() { ensureUnsealed(); diff --git a/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.h b/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.h index aa4ed58df..22d91300b 100644 --- a/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.h +++ b/ReactCommon/fabric/components/scrollview/ScrollViewShadowNode.h @@ -7,8 +7,6 @@ #pragma once -#include - #include #include #include @@ -17,22 +15,22 @@ namespace facebook { namespace react { -class ScrollViewShadowNode; - -using SharedScrollViewShadowNode = std::shared_ptr; +extern const char ScrollViewComponentName[]; /* * `ShadowNode` for component. */ class ScrollViewShadowNode final: - public ConcreteViewShadowNode { + public ConcreteViewShadowNode< + ScrollViewComponentName, + ScrollViewProps, + ScrollViewEventEmitter + > { public: using ConcreteViewShadowNode::ConcreteViewShadowNode; - ComponentName getComponentName() const override; - #pragma mark - LayoutableShadowNode void layout(LayoutContext layoutContext) override; diff --git a/ReactCommon/fabric/components/switch/SwitchShadowNode.cpp b/ReactCommon/fabric/components/switch/SwitchShadowNode.cpp index 87896d0c6..9324baea2 100644 --- a/ReactCommon/fabric/components/switch/SwitchShadowNode.cpp +++ b/ReactCommon/fabric/components/switch/SwitchShadowNode.cpp @@ -5,14 +5,12 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include "SwitchShadowNode.h" namespace facebook { namespace react { -ComponentName SwitchShadowNode::getComponentName() const { - return ComponentName("Switch"); -} +extern const char SwitchComponentName[] = "Switch"; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/switch/SwitchShadowNode.h b/ReactCommon/fabric/components/switch/SwitchShadowNode.h index be4d6f5e7..076b9af9f 100644 --- a/ReactCommon/fabric/components/switch/SwitchShadowNode.h +++ b/ReactCommon/fabric/components/switch/SwitchShadowNode.h @@ -14,18 +14,17 @@ namespace facebook { namespace react { +extern const char SwitchComponentName[]; + /* * `ShadowNode` for component. */ -class SwitchShadowNode final: - public ConcreteViewShadowNode { - -public: - - using ConcreteViewShadowNode::ConcreteViewShadowNode; - - ComponentName getComponentName() const override; -}; +using SwitchShadowNode = + ConcreteViewShadowNode< + SwitchComponentName, + SwitchProps, + SwitchEventEmitter + >; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp index 6f7c0333b..1e6960a59 100644 --- a/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp +++ b/ReactCommon/fabric/components/text/basetext/BaseTextShadowNode.cpp @@ -24,7 +24,7 @@ AttributedString BaseTextShadowNode::getAttributedString( for (const auto &childNode : *childNodes) { // RawShadowNode - SharedRawTextShadowNode rawTextShadowNode = std::dynamic_pointer_cast(childNode); + auto rawTextShadowNode = std::dynamic_pointer_cast(childNode); if (rawTextShadowNode) { AttributedString::Fragment fragment; fragment.string = rawTextShadowNode->getProps()->text; @@ -34,7 +34,7 @@ AttributedString BaseTextShadowNode::getAttributedString( } // TextShadowNode - SharedTextShadowNode textShadowNode = std::dynamic_pointer_cast(childNode); + auto textShadowNode = std::dynamic_pointer_cast(childNode); if (textShadowNode) { TextAttributes localTextAttributes = textAttributes; localTextAttributes.apply(textShadowNode->getProps()->textAttributes); diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h b/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h index 3d9074cbc..3ff916b55 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h @@ -30,7 +30,7 @@ public: } void adopt(UnsharedShadowNode shadowNode) const override { - ConcreteComponentDescriptor::adopt(shadowNode); + ConcreteComponentDescriptor::adopt(shadowNode); assert(std::dynamic_pointer_cast(shadowNode)); auto paragraphShadowNode = std::static_pointer_cast(shadowNode); diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphProps.h b/ReactCommon/fabric/components/text/paragraph/ParagraphProps.h index f7e533d20..2f46822cd 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphProps.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphProps.h @@ -18,10 +18,6 @@ namespace facebook { namespace react { -class ParagraphProps; - -using SharedParagraphProps = std::shared_ptr; - /* * Props of component. * Most of the props are directly stored in composed `ParagraphAttributes` diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp index 99222fc31..f249338d9 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp @@ -7,16 +7,12 @@ #include "ParagraphShadowNode.h" -#include - -#import "ParagraphLocalData.h" +#include "ParagraphLocalData.h" namespace facebook { namespace react { -ComponentName ParagraphShadowNode::getComponentName() const { - return ComponentName("Paragraph"); -} +const char ParagraphComponentName[] = "Paragraph"; AttributedString ParagraphShadowNode::getAttributedString() const { if (!cachedAttributedString_.has_value()) { @@ -53,7 +49,7 @@ Size ParagraphShadowNode::measure(LayoutConstraints layoutConstraints) const { void ParagraphShadowNode::layout(LayoutContext layoutContext) { updateLocalData(); - ConcreteViewShadowNode::layout(layoutContext); + ConcreteViewShadowNode::layout(layoutContext); } } // namespace react diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h index bb81b9175..4a1ab6c57 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h @@ -19,9 +19,7 @@ namespace facebook { namespace react { -class ParagraphShadowNode; - -using SharedParagraphShadowNode = std::shared_ptr; +extern const char ParagraphComponentName[]; /* * `ShadowNode` for component, represents -like component @@ -29,15 +27,16 @@ using SharedParagraphShadowNode = std::shared_ptr; * and components. */ class ParagraphShadowNode: - public ConcreteViewShadowNode, + public ConcreteViewShadowNode< + ParagraphComponentName, + ParagraphProps + >, public BaseTextShadowNode { public: using ConcreteViewShadowNode::ConcreteViewShadowNode; - ComponentName getComponentName() const override; - /* * Returns a `AttributedString` which represents text content of the node. */ diff --git a/ReactCommon/fabric/components/text/rawtext/RawTextProps.h b/ReactCommon/fabric/components/text/rawtext/RawTextProps.h index 45f1f3d01..1a473726f 100644 --- a/ReactCommon/fabric/components/text/rawtext/RawTextProps.h +++ b/ReactCommon/fabric/components/text/rawtext/RawTextProps.h @@ -23,7 +23,6 @@ class RawTextProps: public Props { public: - RawTextProps() = default; RawTextProps(const RawTextProps &sourceProps, const RawProps &rawProps); diff --git a/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.cpp b/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.cpp index 730e4aabb..08b4f8353 100644 --- a/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.cpp +++ b/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.cpp @@ -7,14 +7,10 @@ #include "RawTextShadowNode.h" -#include - namespace facebook { namespace react { -ComponentName RawTextShadowNode::getComponentName() const { - return ComponentName("RawText"); -} +extern const char RawTextComponentName[] = "RawText"; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.h b/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.h index 5d37e86e9..e13efdd1f 100644 --- a/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.h +++ b/ReactCommon/fabric/components/text/rawtext/RawTextShadowNode.h @@ -7,18 +7,13 @@ #pragma once -#include - #include #include -#include namespace facebook { namespace react { -class RawTextShadowNode; - -using SharedRawTextShadowNode = std::shared_ptr; +extern const char RawTextComponentName[]; /* * `ShadowNode` for component, represents a purely regular string @@ -26,14 +21,11 @@ using SharedRawTextShadowNode = std::shared_ptr; * is represented as ``. * component must not have any children. */ -class RawTextShadowNode: - public ConcreteShadowNode { - -public: - using ConcreteShadowNode::ConcreteShadowNode; - - ComponentName getComponentName() const override; -}; +using RawTextShadowNode = + ConcreteShadowNode< + RawTextComponentName, + RawTextProps + >; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/text/text/TextProps.h b/ReactCommon/fabric/components/text/text/TextProps.h index 9213b0dfc..35d3a9a95 100644 --- a/ReactCommon/fabric/components/text/text/TextProps.h +++ b/ReactCommon/fabric/components/text/text/TextProps.h @@ -16,16 +16,11 @@ namespace facebook { namespace react { -class TextProps; - -using SharedTextProps = std::shared_ptr; - class TextProps: public Props, public BaseTextProps { public: - TextProps() = default; TextProps(const TextProps &sourceProps, const RawProps &rawProps); diff --git a/ReactCommon/fabric/components/text/text/TextShadowNode.cpp b/ReactCommon/fabric/components/text/text/TextShadowNode.cpp index c49c8bfd2..692388994 100644 --- a/ReactCommon/fabric/components/text/text/TextShadowNode.cpp +++ b/ReactCommon/fabric/components/text/text/TextShadowNode.cpp @@ -7,14 +7,10 @@ #include "TextShadowNode.h" -#include - namespace facebook { namespace react { -ComponentName TextShadowNode::getComponentName() const { - return ComponentName("Text"); -} +extern const char TextComponentName[] = "Text"; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/text/text/TextShadowNode.h b/ReactCommon/fabric/components/text/text/TextShadowNode.h index f9e584f51..b19ddc47f 100644 --- a/ReactCommon/fabric/components/text/text/TextShadowNode.h +++ b/ReactCommon/fabric/components/text/text/TextShadowNode.h @@ -7,30 +7,25 @@ #pragma once -#include -#include #include #include -#include #include -#include namespace facebook { namespace react { -class TextShadowNode; - -using SharedTextShadowNode = std::shared_ptr; +extern const char TextComponentName[]; class TextShadowNode: - public ConcreteShadowNode, + public ConcreteShadowNode< + TextComponentName, + TextProps + >, public BaseTextShadowNode { public: using ConcreteShadowNode::ConcreteShadowNode; - - ComponentName getComponentName() const override; }; } // namespace react diff --git a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h index 0bddc1a7e..e2202e49c 100644 --- a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h @@ -24,9 +24,17 @@ namespace react { * as and similar basic behaviour). * For example: , , but not , . */ -template +template < + const char *concreteComponentName, + typename ViewPropsT = ViewProps, + typename ViewEventEmitterT = ViewEventEmitter +> class ConcreteViewShadowNode: - public ConcreteShadowNode, + public ConcreteShadowNode< + concreteComponentName, + ViewPropsT, + ViewEventEmitterT + >, public AccessibleShadowNode, public YogaLayoutableShadowNode { @@ -50,7 +58,7 @@ public: const SharedShadowNodeSharedList &children, const ShadowNodeCloneFunction &cloneFunction ): - ConcreteShadowNode( + ConcreteShadowNode( tag, rootTag, props, @@ -64,7 +72,7 @@ public: YogaLayoutableShadowNode() { YogaLayoutableShadowNode::setProps(*props); - YogaLayoutableShadowNode::setChildren(ConcreteShadowNode::template getChildrenSlice()); + YogaLayoutableShadowNode::setChildren(ConcreteShadowNode::template getChildrenSlice()); }; ConcreteViewShadowNode( @@ -73,7 +81,7 @@ public: const SharedConcreteViewEventEmitter &eventEmitter, const SharedShadowNodeSharedList &children ): - ConcreteShadowNode( + ConcreteShadowNode( shadowNode, props, eventEmitter, @@ -92,7 +100,7 @@ public: } if (children) { - YogaLayoutableShadowNode::setChildren(ConcreteShadowNode::template getChildrenSlice()); + YogaLayoutableShadowNode::setChildren(ConcreteShadowNode::template getChildrenSlice()); } }; diff --git a/ReactCommon/fabric/components/view/ViewProps.h b/ReactCommon/fabric/components/view/ViewProps.h index d9d9e6143..d245e1c5e 100644 --- a/ReactCommon/fabric/components/view/ViewProps.h +++ b/ReactCommon/fabric/components/view/ViewProps.h @@ -27,7 +27,6 @@ class ViewProps: public AccessibilityProps { public: - ViewProps() = default; ViewProps(const YGStyle &yogaStyle); ViewProps(const ViewProps &sourceProps, const RawProps &rawProps); diff --git a/ReactCommon/fabric/components/view/ViewShadowNode.cpp b/ReactCommon/fabric/components/view/ViewShadowNode.cpp index 2bf7f30d7..a633c6869 100644 --- a/ReactCommon/fabric/components/view/ViewShadowNode.cpp +++ b/ReactCommon/fabric/components/view/ViewShadowNode.cpp @@ -10,9 +10,7 @@ namespace facebook { namespace react { -ComponentName ViewShadowNode::getComponentName() const { - return ComponentName("View"); -} +const char ViewComponentName[] = "View"; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/components/view/ViewShadowNode.h b/ReactCommon/fabric/components/view/ViewShadowNode.h index 8fa20ebad..449e6c0f4 100644 --- a/ReactCommon/fabric/components/view/ViewShadowNode.h +++ b/ReactCommon/fabric/components/view/ViewShadowNode.h @@ -7,27 +7,20 @@ #pragma once -#include - #include #include namespace facebook { namespace react { -class ViewShadowNode; +extern const char ViewComponentName[]; -using SharedViewShadowNode = std::shared_ptr; - -class ViewShadowNode final: - public ConcreteViewShadowNode { - -public: - - using ConcreteViewShadowNode::ConcreteViewShadowNode; - - ComponentName getComponentName() const override; -}; +using ViewShadowNode = + ConcreteViewShadowNode< + ViewComponentName, + ViewProps, + ViewEventEmitter + >; } // namespace react } // namespace facebook diff --git a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h index 09028d3a1..2f607fd08 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h @@ -37,22 +37,11 @@ public: eventDispatcher_(eventDispatcher) {} ComponentHandle getComponentHandle() const override { - return typeid(ShadowNodeT).hash_code(); + return ShadowNodeT::Handle(); } ComponentName getComponentName() const override { - // Even if this looks suboptimal, it is the only way to call - // a virtual non-static method of `ShadowNodeT`. - // Because it is not a hot path (it is executed once per an app run), - // it's fine. - return std::make_shared( - 0, - 0, - std::make_shared(), - nullptr, - ShadowNode::emptySharedShadowNodeSharedList(), - nullptr - )->ShadowNodeT::getComponentName(); + return ShadowNodeT::Name(); } SharedShadowNode createShadowNode( @@ -74,6 +63,7 @@ public: ); adopt(shadowNode); + return shadowNode; } @@ -123,6 +113,7 @@ protected: virtual void adopt(UnsharedShadowNode shadowNode) const { // Default implementation does nothing. + assert(shadowNode->getComponentHandle() == getComponentHandle()); } private: diff --git a/ReactCommon/fabric/core/primitives/ReactPrimitives.h b/ReactCommon/fabric/core/primitives/ReactPrimitives.h index 065c761c9..b498f1a52 100644 --- a/ReactCommon/fabric/core/primitives/ReactPrimitives.h +++ b/ReactCommon/fabric/core/primitives/ReactPrimitives.h @@ -35,7 +35,7 @@ using SharedRawProps = std::shared_ptr; * Practically, it's something that concrete ShadowNode and concrete * ComponentDescriptor have in common. */ -using ComponentHandle = size_t; +using ComponentHandle = int64_t; /* * String identifier for components used for addressing them from diff --git a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h index cc6b7ebb0..01568d583 100644 --- a/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ConcreteShadowNode.h @@ -19,7 +19,11 @@ namespace react { * `ConcreteShadowNode` is a default implementation of `ShadowNode` interface * with many handy features. */ -template +template < + const char *concreteComponentName, + typename PropsT, + typename EventEmitterT = EventEmitter +> class ConcreteShadowNode: public ShadowNode { static_assert(std::is_base_of::value, "PropsT must be a descendant of Props"); @@ -30,6 +34,14 @@ public: using SharedConcreteEventEmitter = std::shared_ptr; using SharedConcreteShadowNode = std::shared_ptr; + static ComponentName Name() { + return ComponentName(concreteComponentName); + } + + static ComponentHandle Handle() { + return ComponentHandle(concreteComponentName); + } + static SharedConcreteProps Props(const RawProps &rawProps, const SharedProps &baseProps = nullptr) { return std::make_shared(baseProps ? *std::static_pointer_cast(baseProps) : PropsT(), rawProps); } @@ -69,8 +81,12 @@ public: children ) {} - virtual ComponentHandle getComponentHandle() const { - return typeid(*this).hash_code(); + ComponentName getComponentName() const override { + return ComponentName(concreteComponentName); + } + + ComponentHandle getComponentHandle() const override { + return reinterpret_cast(concreteComponentName); } const SharedConcreteProps getProps() const { diff --git a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp index 12e09787a..53aa9ca6b 100644 --- a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp +++ b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp @@ -14,7 +14,8 @@ using namespace facebook::react; TEST(ComponentDescriptorTest, createShadowNode) { SharedComponentDescriptor descriptor = std::make_shared(nullptr); - ASSERT_EQ(descriptor->getComponentHandle(), typeid(TestShadowNode).hash_code()); + ASSERT_EQ(descriptor->getComponentHandle(), TestShadowNode::Handle()); + ASSERT_STREQ(descriptor->getComponentName().c_str(), TestShadowNode::Name().c_str()); ASSERT_STREQ(descriptor->getComponentName().c_str(), "Test"); RawProps raw; @@ -22,7 +23,8 @@ TEST(ComponentDescriptorTest, createShadowNode) { SharedProps props = descriptor->cloneProps(nullptr, raw); SharedShadowNode node = descriptor->createShadowNode(9, 1, descriptor->createEventEmitter(0, 9), props); - ASSERT_EQ(node->getComponentHandle(), typeid(TestShadowNode).hash_code()); + ASSERT_EQ(node->getComponentHandle(), TestShadowNode::Handle()); + ASSERT_STREQ(node->getComponentName().c_str(), TestShadowNode::Name().c_str()); ASSERT_STREQ(node->getComponentName().c_str(), "Test"); ASSERT_EQ(node->getTag(), 9); ASSERT_EQ(node->getRootTag(), 1); @@ -38,7 +40,6 @@ TEST(ComponentDescriptorTest, cloneShadowNode) { SharedShadowNode node = descriptor->createShadowNode(9, 1, descriptor->createEventEmitter(0, 9), props); SharedShadowNode cloned = descriptor->cloneShadowNode(node); - ASSERT_EQ(cloned->getComponentHandle(), typeid(TestShadowNode).hash_code()); ASSERT_STREQ(cloned->getComponentName().c_str(), "Test"); ASSERT_EQ(cloned->getTag(), 9); ASSERT_EQ(cloned->getRootTag(), 1); diff --git a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp index ddb57c8fc..aa058375d 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeTest.cpp @@ -35,8 +35,6 @@ TEST(ShadowNodeTest, handleShadowNodeCreation) { ASSERT_EQ(node->getTag(), 9); ASSERT_EQ(node->getRootTag(), 1); ASSERT_EQ(node->getEventEmitter(), nullptr); - TestShadowNode *nodePtr = node.get(); - ASSERT_EQ(node->getComponentHandle(), typeid(*nodePtr).hash_code()); ASSERT_EQ(node->getChildren()->size(), 0); ASSERT_STREQ(node->getProps()->nativeId.c_str(), "testNativeID"); diff --git a/ReactCommon/fabric/core/tests/TestComponent.h b/ReactCommon/fabric/core/tests/TestComponent.h index 8659b960c..22c2c32e1 100644 --- a/ReactCommon/fabric/core/tests/TestComponent.h +++ b/ReactCommon/fabric/core/tests/TestComponent.h @@ -36,6 +36,8 @@ private: int number_ {0}; }; +static const char TestComponentName[] = "Test"; + class TestProps : public Props { public: using Props::Props; @@ -46,21 +48,12 @@ using SharedTestProps = std::shared_ptr; class TestShadowNode; using SharedTestShadowNode = std::shared_ptr; -class TestShadowNode : public ConcreteShadowNode { +class TestShadowNode : public ConcreteShadowNode { public: using ConcreteShadowNode::ConcreteShadowNode; - - ComponentName getComponentName() const override { - return ComponentName("Test"); - } }; class TestComponentDescriptor: public ConcreteComponentDescriptor { public: using ConcreteComponentDescriptor::ConcreteComponentDescriptor; - - // TODO (shergin): Why does this gets repeated here and the shadow node class? - ComponentName getComponentName() const override { - return "Test"; - } };