From b39c75f20e0fc604c26cfffa3e5bad2822c4c1f1 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Thu, 30 Jan 2020 19:43:33 -0800 Subject: [PATCH] Fabric: `Element<>` now returns (and accepts) mutable (non const) shared pointers Summary: It makes perfect sense because `Builder` builds totally new shadow trees, so those are not sealed or used by anyone yet. Assigning it to const shared pointer will do logical sealing (and it does not requires const-cast). Fewer const-casts in the code, fewer bugs. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D19596284 fbshipit-source-id: 75d1c706034958ba7e4bc80a68af75a57c46eb6f --- .../fabric/core/tests/ShadowNodeFamilyTest.cpp | 4 ++-- ReactCommon/fabric/element/ComponentBuilder.cpp | 8 +++++--- ReactCommon/fabric/element/ComponentBuilder.h | 7 +++---- ReactCommon/fabric/element/Element.h | 14 +++++++------- ReactCommon/fabric/element/ElementFragment.h | 2 +- ReactCommon/fabric/element/tests/ElementTest.cpp | 8 ++++---- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp index 7adc67b640..b5737b4b24 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp @@ -36,8 +36,8 @@ TEST(ShadowNodeFamilyTest, sealObjectCorrectly) { auto builder = ComponentBuilder{componentDescriptorRegistry}; - auto shadowNodeAAA = std::shared_ptr{}; - auto shadowNodeAA = std::shared_ptr{}; + auto shadowNodeAAA = std::shared_ptr{}; + auto shadowNodeAA = std::shared_ptr{}; // clang-format off auto elementA = diff --git a/ReactCommon/fabric/element/ComponentBuilder.cpp b/ReactCommon/fabric/element/ComponentBuilder.cpp index 7c59f7a63c..b0ffe7075a 100644 --- a/ReactCommon/fabric/element/ComponentBuilder.cpp +++ b/ReactCommon/fabric/element/ComponentBuilder.cpp @@ -14,7 +14,7 @@ ComponentBuilder::ComponentBuilder( ComponentDescriptorRegistry::Shared const &componentDescriptorRegistry) : componentDescriptorRegistry_(componentDescriptorRegistry){}; -ShadowNode::Shared ComponentBuilder::build( +ShadowNode::Unshared ComponentBuilder::build( ElementFragment const &elementFragment) const { auto &componentDescriptor = componentDescriptorRegistry_->at(elementFragment.componentHandle); @@ -30,19 +30,21 @@ ShadowNode::Shared ComponentBuilder::build( elementFragment.tag, elementFragment.surfaceId, nullptr}, nullptr); - auto shadowNode = componentDescriptor.createShadowNode( + auto constShadowNode = componentDescriptor.createShadowNode( ShadowNodeFragment{ elementFragment.props, std::make_shared(children), elementFragment.state}, family); + auto shadowNode = std::const_pointer_cast(constShadowNode); + if (elementFragment.referenceCallback) { elementFragment.referenceCallback(shadowNode); } if (elementFragment.finalizeCallback) { - elementFragment.finalizeCallback(const_cast(*shadowNode)); + elementFragment.finalizeCallback(*shadowNode); } return shadowNode; diff --git a/ReactCommon/fabric/element/ComponentBuilder.h b/ReactCommon/fabric/element/ComponentBuilder.h index d33a3934c2..9e015665ec 100644 --- a/ReactCommon/fabric/element/ComponentBuilder.h +++ b/ReactCommon/fabric/element/ComponentBuilder.h @@ -42,16 +42,15 @@ class ComponentBuilder final { * `ComponentDescriptorRegistry`. */ template - std::shared_ptr build(Element element) const { - return std::static_pointer_cast( - build(element.fragment_)); + std::shared_ptr build(Element element) const { + return std::static_pointer_cast(build(element.fragment_)); } private: /* * Internal, type-erased version of `build`. */ - ShadowNode::Shared build(ElementFragment const &elementFragment) const; + ShadowNode::Unshared build(ElementFragment const &elementFragment) const; ComponentDescriptorRegistry::Shared componentDescriptorRegistry_; }; diff --git a/ReactCommon/fabric/element/Element.h b/ReactCommon/fabric/element/Element.h index d1056b56f0..4f7f2e0e77 100644 --- a/ReactCommon/fabric/element/Element.h +++ b/ReactCommon/fabric/element/Element.h @@ -31,10 +31,10 @@ template class Element final { public: using ConcreteProps = typename ShadowNodeT::ConcreteProps; - using SharedConcreteProps = typename ShadowNodeT::SharedConcreteProps; - using ConcreteEventEmitter = typename ShadowNodeT::ConcreteEventEmitter; + using SharedConcreteProps = std::shared_ptr; + using UnsharedConcreteProps = std::shared_ptr; using ConcreteShadowNode = ShadowNodeT; - using ConcreteSharedShadowNode = std::shared_ptr; + using ConcreteUnsharedShadowNode = std::shared_ptr; using ConcreteReferenceCallback = std::function const &shadowNode)>; @@ -105,7 +105,7 @@ class Element final { * component which is being constructed. */ Element &reference( - std::function + std::function callback) { fragment_.referenceCallback = callback; return *this; @@ -115,10 +115,10 @@ class Element final { * During component construction, assigns a given pointer to a component * that is being constructed. */ - Element &reference(ConcreteSharedShadowNode &inShadowNode) { + Element &reference(ConcreteUnsharedShadowNode &outShadowNode) { fragment_.referenceCallback = [&](ShadowNode::Shared const &shadowNode) { - inShadowNode = - std::static_pointer_cast(shadowNode); + outShadowNode = std::const_pointer_cast( + std::static_pointer_cast(shadowNode)); }; return *this; } diff --git a/ReactCommon/fabric/element/ElementFragment.h b/ReactCommon/fabric/element/ElementFragment.h index f5f5c9acb6..b1438698a4 100644 --- a/ReactCommon/fabric/element/ElementFragment.h +++ b/ReactCommon/fabric/element/ElementFragment.h @@ -28,7 +28,7 @@ class ElementFragment final { using List = std::vector; using ListOfShared = std::vector; using ReferenceCallback = - std::function; + std::function; using FinalizeCallback = std::function; /* diff --git a/ReactCommon/fabric/element/tests/ElementTest.cpp b/ReactCommon/fabric/element/tests/ElementTest.cpp index c3ecc1a642..0a0a09b015 100644 --- a/ReactCommon/fabric/element/tests/ElementTest.cpp +++ b/ReactCommon/fabric/element/tests/ElementTest.cpp @@ -21,10 +21,10 @@ using namespace facebook::react; TEST(ElementTest, testNormalCases) { auto builder = simpleComponentBuilder(); - auto shadowNodeA = std::shared_ptr{}; - auto shadowNodeAA = std::shared_ptr{}; - auto shadowNodeAB = std::shared_ptr{}; - auto shadowNodeABA = std::shared_ptr{}; + auto shadowNodeA = std::shared_ptr{}; + auto shadowNodeAA = std::shared_ptr{}; + auto shadowNodeAB = std::shared_ptr{}; + auto shadowNodeABA = std::shared_ptr{}; auto propsAA = std::make_shared(); propsAA->nativeId = "node AA";