From 7f79b46badeec8a41625bc2f09d270374015fdd7 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 28 Jan 2020 09:27:56 -0800 Subject: [PATCH] Initialise ShadowNodeFamily before ShadowNode is created Summary: Changelog: [internal] 1. Use `ShadowNode::Shared` instead of `SharedShadowNode`. 2. Initialise `ShadowNodeFamily` before `ShadowNode`. Why? This is a step in order to merge `StateCoordinator` into `ShadowNodeFamily` and use it as target for state updates. Reviewed By: shergin Differential Revision: D19471399 fbshipit-source-id: 2f67901c901349d238c711f9eeaadb19fe7c1110 --- ReactCommon/fabric/core/BUCK | 1 + .../componentdescriptor/ComponentDescriptor.h | 4 +- .../ConcreteComponentDescriptor.h | 8 +- .../core/shadownode/ShadowNodeFamily.cpp | 4 + .../fabric/core/shadownode/ShadowNodeFamily.h | 2 + .../core/tests/ComponentDescriptorTest.cpp | 46 +++---- .../core/tests/ShadowNodeFamilyTest.cpp | 115 +++++++----------- .../fabric/element/ComponentBuilder.cpp | 8 +- ReactCommon/fabric/mounting/ShadowTree.cpp | 5 +- .../uimanager/ComponentDescriptorRegistry.cpp | 10 +- ReactCommon/fabric/uimanager/UIManager.cpp | 10 +- 11 files changed, 100 insertions(+), 113 deletions(-) diff --git a/ReactCommon/fabric/core/BUCK b/ReactCommon/fabric/core/BUCK index d17beea3078..4736155f99a 100644 --- a/ReactCommon/fabric/core/BUCK +++ b/ReactCommon/fabric/core/BUCK @@ -84,6 +84,7 @@ fb_xplat_cxx_test( platforms = (ANDROID, APPLE, CXX), deps = [ "fbsource//xplat/folly:molly", + "fbsource//xplat/js/react-native-github/ReactCommon/fabric/element:element", "fbsource//xplat/third-party/gmock:gtest", react_native_xplat_target("fabric/components/view:view"), ":core", diff --git a/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h index 74b24026909..e1edd533b4d 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ComponentDescriptor.h @@ -75,9 +75,9 @@ class ComponentDescriptor { /* * Creates a new `ShadowNode` of a particular component type. */ - virtual SharedShadowNode createShadowNode( + virtual ShadowNode::Shared createShadowNode( const ShadowNodeFragment &fragment, - ShadowNodeFamilyFragment const &familyFragment) const = 0; + ShadowNodeFamily::Shared const &family) const = 0; /* * Clones a `ShadowNode` with optionally new `props` and/or `children`. diff --git a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h index 24ba7656a9a..f66574a97c7 100644 --- a/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h +++ b/ReactCommon/fabric/core/componentdescriptor/ConcreteComponentDescriptor.h @@ -61,14 +61,10 @@ class ConcreteComponentDescriptor : public ComponentDescriptor { return ShadowNodeT::BaseTraits(); } - SharedShadowNode createShadowNode( + ShadowNode::Shared createShadowNode( const ShadowNodeFragment &fragment, - ShadowNodeFamilyFragment const &familyFragment) const override { + ShadowNodeFamily::Shared const &family) const override { assert(std::dynamic_pointer_cast(fragment.props)); - assert(std::dynamic_pointer_cast( - familyFragment.eventEmitter)); - - auto family = std::make_shared(familyFragment, *this); auto shadowNode = std::make_shared(fragment, family, getTraits()); diff --git a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp index d96bae1a6eb..fe5a5b82ec9 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp +++ b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.cpp @@ -39,6 +39,10 @@ ComponentHandle ShadowNodeFamily::getComponentHandle() const { return componentHandle_; } +SurfaceId ShadowNodeFamily::getSurfaceId() const { + return surfaceId_; +} + ComponentName ShadowNodeFamily::getComponentName() const { return componentName_; } diff --git a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h index f4ff7169edb..160188a2894 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNodeFamily.h @@ -62,6 +62,8 @@ class ShadowNodeFamily { */ AncestorList getAncestors(ShadowNode const &ancestorShadowNode) const; + SurfaceId getSurfaceId() const; + private: friend ShadowNode; diff --git a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp index ca951a8eac1..a695d558be7 100644 --- a/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp +++ b/ReactCommon/fabric/core/tests/ComponentDescriptorTest.cpp @@ -23,16 +23,15 @@ TEST(ComponentDescriptorTest, createShadowNode) { const auto &raw = RawProps(folly::dynamic::object("nativeID", "abc")); SharedProps props = descriptor->cloneProps(nullptr, raw); + auto family = std::make_shared( + ShadowNodeFamilyFragment{9, 1, descriptor->createEventEmitter(0, 9)}, + *descriptor); SharedShadowNode node = descriptor->createShadowNode( ShadowNodeFragment{ /* .props = */ props, }, - ShadowNodeFamilyFragment{ - /* .tag = */ 9, - /* .surfaceId = */ 1, - /* .eventEmitter = */ descriptor->createEventEmitter(0, 9), - }); + family); EXPECT_EQ(node->getComponentHandle(), TestShadowNode::Handle()); EXPECT_STREQ(node->getComponentName(), TestShadowNode::Name()); @@ -50,15 +49,14 @@ TEST(ComponentDescriptorTest, cloneShadowNode) { const auto &raw = RawProps(folly::dynamic::object("nativeID", "abc")); SharedProps props = descriptor->cloneProps(nullptr, raw); + auto family = std::make_shared( + ShadowNodeFamilyFragment{9, 1, descriptor->createEventEmitter(0, 9)}, + *descriptor); SharedShadowNode node = descriptor->createShadowNode( ShadowNodeFragment{ /* .props = */ props, }, - ShadowNodeFamilyFragment{ - /* .tag = */ 9, - /* .surfaceId = */ 1, - /* .eventEmitter = */ descriptor->createEventEmitter(0, 9), - }); + family); SharedShadowNode cloned = descriptor->cloneShadowNode(*node, {}); EXPECT_STREQ(cloned->getComponentName(), "Test"); @@ -75,34 +73,30 @@ TEST(ComponentDescriptorTest, appendChild) { const auto &raw = RawProps(folly::dynamic::object("nativeID", "abc")); SharedProps props = descriptor->cloneProps(nullptr, raw); - + auto family1 = std::make_shared( + ShadowNodeFamilyFragment{1, 1, descriptor->createEventEmitter(0, 9)}, + *descriptor); SharedShadowNode node1 = descriptor->createShadowNode( ShadowNodeFragment{ /* .props = */ props, }, - ShadowNodeFamilyFragment{ - /* .tag = */ 1, - /* .surfaceId = */ 1, - /* .eventEmitter = */ descriptor->createEventEmitter(0, 9), - }); + family1); + auto family2 = std::make_shared( + ShadowNodeFamilyFragment{2, 1, descriptor->createEventEmitter(0, 2)}, + *descriptor); SharedShadowNode node2 = descriptor->createShadowNode( ShadowNodeFragment{ /* .props = */ props, }, - ShadowNodeFamilyFragment{ - /* .tag = */ 2, - /* .surfaceId = */ 1, - /* .eventEmitter = */ descriptor->createEventEmitter(0, 9), - }); + family2); + auto family3 = std::make_shared( + ShadowNodeFamilyFragment{3, 1, descriptor->createEventEmitter(0, 3)}, + *descriptor); SharedShadowNode node3 = descriptor->createShadowNode( ShadowNodeFragment{ /* .props = */ props, }, - ShadowNodeFamilyFragment{ - /* .tag = */ 3, - /* .surfaceId = */ 1, - /* .eventEmitter = */ descriptor->createEventEmitter(0, 9), - }); + family3); descriptor->appendChild(node1, node2); descriptor->appendChild(node1, node3); diff --git a/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp b/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp index bd06d26d001..7adc67b6404 100644 --- a/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp +++ b/ReactCommon/fabric/core/tests/ShadowNodeFamilyTest.cpp @@ -8,7 +8,11 @@ #include #include -#include "TestComponent.h" + +#include +#include +#include +#include using namespace facebook::react; @@ -21,84 +25,55 @@ TEST(ShadowNodeFamilyTest, sealObjectCorrectly) { * * */ - SurfaceId surfaceId = 1; - auto eventDispatcher = std::shared_ptr(); - auto componentDescriptor = TestComponentDescriptor({eventDispatcher}); - auto props = std::make_shared(); + ComponentDescriptorProviderRegistry componentDescriptorProviderRegistry{}; + auto eventDispatcher = EventDispatcher::Shared{}; + auto componentDescriptorRegistry = + componentDescriptorProviderRegistry.createComponentDescriptorRegistry( + ComponentDescriptorParameters{eventDispatcher, nullptr, nullptr}); - auto familyAAA = std::make_shared( - ShadowNodeFamilyFragment{ - /* .tag = */ 12, - /* .surfaceId = */ surfaceId, - /* .eventEmitter = */ nullptr, - }, - componentDescriptor); + componentDescriptorProviderRegistry.add( + concreteComponentDescriptorProvider()); - auto nodeAAA = std::make_shared( - ShadowNodeFragment{ - /* .props = */ props, - /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), - }, - familyAAA, - ShadowNodeTraits{}); + auto builder = ComponentBuilder{componentDescriptorRegistry}; - auto nodeAAChildren = - std::make_shared(SharedShadowNodeList{nodeAAA}); - auto familyAA = std::make_shared( - ShadowNodeFamilyFragment{ - /* .tag = */ 11, - /* .surfaceId = */ surfaceId, - /* .eventEmitter = */ nullptr, - }, - componentDescriptor); - auto nodeAA = std::make_shared( - ShadowNodeFragment{ - /* .props = */ props, - /* .children = */ nodeAAChildren, - }, - familyAA, - ShadowNodeTraits{}); + auto shadowNodeAAA = std::shared_ptr{}; + auto shadowNodeAA = std::shared_ptr{}; - auto nodeAChildren = - std::make_shared(SharedShadowNodeList{nodeAA}); + // clang-format off + auto elementA = + Element() + .tag(1) + .finalize([](ViewShadowNode &shadowNode){ + shadowNode.sealRecursive(); + }) + .children({ + Element() + .tag(2) + .reference(shadowNodeAA) + .children({ + Element() + .reference(shadowNodeAAA) + .tag(3) + }) + }); + auto elementB = + Element() + .tag(1) + .finalize([](ViewShadowNode &shadowNode){ + shadowNode.sealRecursive(); + }); + // clang-format on - auto familyA = std::make_shared( - ShadowNodeFamilyFragment{ - /* .tag = */ 17, - /* .surfaceId = */ surfaceId, - /* .eventEmitter = */ nullptr, - }, - componentDescriptor); - auto nodeA = std::make_shared( - ShadowNodeFragment{ - /* .props = */ props, - /* .children = */ nodeAChildren, - }, - familyA, - ShadowNodeTraits{}); - - auto familyZ = std::make_shared( - ShadowNodeFamilyFragment{ - /* .tag = */ 18, - /* .surfaceId = */ surfaceId, - /* .eventEmitter = */ nullptr, - }, - componentDescriptor); - auto nodeZ = std::make_shared( - ShadowNodeFragment{ - /* .props = */ props, - /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), - }, - familyZ, - ShadowNodeTraits{}); + auto shadowNodeA = builder.build(elementA); + auto shadowNodeB = builder.build(elementB); // Negative case: - auto ancestors1 = nodeZ->getFamily().getAncestors(*nodeA); + auto ancestors1 = shadowNodeB->getFamily().getAncestors(*shadowNodeA); EXPECT_EQ(ancestors1.size(), 0); // Positive case: - auto ancestors2 = nodeAAA->getFamily().getAncestors(*nodeA); + auto ancestors2 = shadowNodeAAA->getFamily().getAncestors(*shadowNodeA); EXPECT_EQ(ancestors2.size(), 2); - EXPECT_EQ(&ancestors2[0].first.get(), nodeA.get()); - EXPECT_EQ(&ancestors2[1].first.get(), nodeAA.get()); + EXPECT_EQ(&ancestors2[0].first.get(), shadowNodeA.get()); + EXPECT_EQ(&ancestors2[1].first.get(), shadowNodeAA.get()); } diff --git a/ReactCommon/fabric/element/ComponentBuilder.cpp b/ReactCommon/fabric/element/ComponentBuilder.cpp index 6424b1b6fc6..d42d6d40912 100644 --- a/ReactCommon/fabric/element/ComponentBuilder.cpp +++ b/ReactCommon/fabric/element/ComponentBuilder.cpp @@ -28,13 +28,17 @@ ShadowNode::Shared ComponentBuilder::build( auto eventEmitter = componentDescriptor.createEventEmitter(nullptr, elementFragment.tag); + auto family = std::make_shared( + ShadowNodeFamilyFragment{ + elementFragment.tag, elementFragment.surfaceId, eventEmitter}, + componentDescriptor); + auto shadowNode = componentDescriptor.createShadowNode( ShadowNodeFragment{ elementFragment.props, std::make_shared(children), elementFragment.state}, - ShadowNodeFamilyFragment{ - elementFragment.tag, elementFragment.surfaceId, eventEmitter}); + family); if (elementFragment.referenceCallback) { elementFragment.referenceCallback(shadowNode); diff --git a/ReactCommon/fabric/mounting/ShadowTree.cpp b/ReactCommon/fabric/mounting/ShadowTree.cpp index dca5a27da8c..af002b0f628 100644 --- a/ReactCommon/fabric/mounting/ShadowTree.cpp +++ b/ReactCommon/fabric/mounting/ShadowTree.cpp @@ -102,12 +102,15 @@ ShadowTree::ShadowTree( const auto props = std::make_shared( *RootShadowNode::defaultSharedProps(), layoutConstraints, layoutContext); + auto family = std::make_shared( + ShadowNodeFamilyFragment{surfaceId, surfaceId, noopEventEmitter}, + rootComponentDescriptor); rootShadowNode_ = std::static_pointer_cast( rootComponentDescriptor.createShadowNode( ShadowNodeFragment{ /* .props = */ props, }, - {surfaceId, surfaceId, noopEventEmitter})); + family)); mountingCoordinator_ = std::make_shared( ShadowTreeRevision{rootShadowNode_, 0, {}}); diff --git a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp index 298d6d5b218..a935232509e 100644 --- a/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp +++ b/ReactCommon/fabric/uimanager/ComponentDescriptorRegistry.cpp @@ -172,8 +172,12 @@ SharedShadowNode ComponentDescriptorRegistry::createNode( auto unifiedComponentName = componentNameByReactViewName(viewName); auto const &componentDescriptor = this->at(unifiedComponentName); - auto const eventEmitter = - componentDescriptor.createEventEmitter(std::move(eventTarget), tag); + auto family = std::make_shared( + ShadowNodeFamilyFragment{ + tag, + surfaceId, + componentDescriptor.createEventEmitter(std::move(eventTarget), tag)}, + componentDescriptor); auto const props = componentDescriptor.cloneProps(nullptr, RawProps(propsDynamic)); auto const state = componentDescriptor.createInitialState( @@ -185,7 +189,7 @@ SharedShadowNode ComponentDescriptorRegistry::createNode( /* .children = */ ShadowNodeFragment::childrenPlaceholder(), /* .state = */ state, }, - {tag, surfaceId, eventEmitter}); + family); } void ComponentDescriptorRegistry::setFallbackComponentDescriptor( diff --git a/ReactCommon/fabric/uimanager/UIManager.cpp b/ReactCommon/fabric/uimanager/UIManager.cpp index 3dfc80e6e1a..d748ce4b2ee 100644 --- a/ReactCommon/fabric/uimanager/UIManager.cpp +++ b/ReactCommon/fabric/uimanager/UIManager.cpp @@ -33,8 +33,12 @@ SharedShadowNode UIManager::createNode( auto fallbackDescriptor = componentDescriptorRegistry_->getFallbackComponentDescriptor(); - auto const eventEmitter = - componentDescriptor.createEventEmitter(std::move(eventTarget), tag); + auto family = std::make_shared( + ShadowNodeFamilyFragment{ + tag, + surfaceId, + componentDescriptor.createEventEmitter(std::move(eventTarget), tag)}, + componentDescriptor); auto const props = componentDescriptor.cloneProps(nullptr, rawProps); auto const state = componentDescriptor.createInitialState( ShadowNodeFragment{props}, surfaceId); @@ -51,7 +55,7 @@ SharedShadowNode UIManager::createNode( /* .children = */ ShadowNodeFragment::childrenPlaceholder(), /* .state = */ state, }, - ShadowNodeFamilyFragment{tag, surfaceId, eventEmitter}); + family); // state->commit(x) associates a ShadowNode with the State object. // state->commit(x) must be called before calling updateState; updateState