diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp index 4ff280cf23ee..7c7c8ded57d3 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -139,12 +139,23 @@ std::shared_ptr ShadowNode::clone( const auto& family = *family_; const auto& componentDescriptor = family.componentDescriptor_; - std::optional propsOverride; + std::shared_ptr props; { std::lock_guard lock(family.nativePropsMutex); - if (family.nativeProps_DEPRECATED != nullptr) { + const auto& nativeProps = family.nativeProps_DEPRECATED; + if (nativeProps != nullptr) { if (fragment.props == ShadowNodeFragment::propsPlaceholder()) { - propsOverride.emplace(*family.nativeProps_DEPRECATED); + // Props that already include `nativeProps_DEPRECATED` don't need + // them applied again. + if (nativeProps->appliedProps != props_) { + // Clone existing `props_` with `family.nativeProps_DEPRECATED` to + // apply previously set props via `setNativeProps` API. + auto propsParserContext = + PropsParserContext{family_->getSurfaceId(), {}}; + props = componentDescriptor.cloneProps( + propsParserContext, props_, RawProps(nativeProps->props)); + nativeProps->appliedProps = props; + } } else { // TODO: We might need to merge fragment.props with // `family.nativeProps_DEPRECATED`. @@ -152,16 +163,10 @@ std::shared_ptr ShadowNode::clone( } } - if (propsOverride) { - // Clone existing `props_` with `family.nativeProps_DEPRECATED` to - // apply previously set props via `setNativeProps` API. The parsed - // result escapes the lock so the rest of the clone work runs - // unblocked. - auto propsParserContext = PropsParserContext{family_->getSurfaceId(), {}}; + if (props) { return componentDescriptor.cloneShadowNode( *this, - {.props = componentDescriptor.cloneProps( - propsParserContext, props_, std::move(*propsOverride)), + {.props = std::move(props), .children = fragment.children, .state = fragment.state}); } diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h index 2ffc2ce5ac2a..eb1626ccae48 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h @@ -19,6 +19,7 @@ namespace facebook::react { class ComponentDescriptor; +class Props; class ShadowNode; class State; @@ -122,12 +123,25 @@ class ShadowNodeFamily final : public jsi::NativeState { */ void dispatchRawState(StateUpdate &&stateUpdate, EventQueue::UpdateMode updateMode); + struct NativeProps_DEPRECATED { + explicit NativeProps_DEPRECATED(folly::dynamic props) : props(std::move(props)) {} + + folly::dynamic props; + + /* + * The props of a shadow node that already include `props`, so that + * cloning that node doesn't apply them again. + */ + std::shared_ptr appliedProps = nullptr; + }; + /* * Holds currently applied native props. `nullptr` if setNativeProps API is * not used. It is used to backport setNativeProps API from the old - * architecture and will be removed in the future. + * architecture and will be removed in the future. Guarded by + * `nativePropsMutex`. */ - mutable std::unique_ptr nativeProps_DEPRECATED; + mutable std::unique_ptr nativeProps_DEPRECATED; mutable std::mutex nativePropsMutex; /** diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp index 7eefe4606177..6a18881fd844 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -146,10 +146,11 @@ std::shared_ptr UIManager::cloneNode( // // The result of this operation is the new `nativeProps_DEPRECATED`. family.nativeProps_DEPRECATED = - std::make_unique(mergeDynamicProps( - *family.nativeProps_DEPRECATED, // source - (folly::dynamic)rawProps, // patch - NullValueStrategy::Ignore)); + std::make_unique( + mergeDynamicProps( + family.nativeProps_DEPRECATED->props, // source + (folly::dynamic)rawProps, // patch + NullValueStrategy::Ignore)); // 2. Compute the final set of props. // @@ -161,7 +162,7 @@ std::shared_ptr UIManager::cloneNode( // We use these finalProps as source of truth for the component. finalProps = mergeDynamicProps( (folly::dynamic)rawProps, // source - *family.nativeProps_DEPRECATED, // patch + family.nativeProps_DEPRECATED->props, // patch NullValueStrategy::Override); } } @@ -471,13 +472,15 @@ void UIManager::setNativeProps_DEPRECATED( // `nativeProps_DEPRECATED` and `rawProps` contain key 'A'. Value from // `rawProps` overrides what was previously in `nativeProps_DEPRECATED`. family.nativeProps_DEPRECATED = - std::make_unique(mergeDynamicProps( - *family.nativeProps_DEPRECATED, - (folly::dynamic)rawProps, - NullValueStrategy::Override)); + std::make_unique( + mergeDynamicProps( + family.nativeProps_DEPRECATED->props, + (folly::dynamic)rawProps, + NullValueStrategy::Override)); } else { family.nativeProps_DEPRECATED = - std::make_unique((folly::dynamic)rawProps); + std::make_unique( + (folly::dynamic)rawProps); } } diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/tests/FabricUIManagerTest.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/tests/FabricUIManagerTest.cpp index 47aaa20ef567..8ae4454f6c69 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/tests/FabricUIManagerTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/tests/FabricUIManagerTest.cpp @@ -155,4 +155,60 @@ TEST_F(FabricUIManagerTest, SetNativePropsMergesWithReactRender) { EXPECT_FLOAT_EQ(merged.opacity, 0.5); } +// Cloning a node whose props already include the props set by +// `setNativeProps_DEPRECATED` (as Yoga does for every child of a node it lays +// out) keeps those props instead of parsing them again. Props set later are +// still applied. +TEST_F(FabricUIManagerTest, CloneAppliesSetNativePropsOnlyOnce) { + uiManager_->setNativeProps_DEPRECATED( + currentViewNode(), RawProps(folly::dynamic::object("opacity", 0.5))); + + auto first = currentViewNode()->clone({}); + EXPECT_FLOAT_EQ( + static_cast(*first->getProps()).opacity, 0.5); + + auto second = first->clone({}); + EXPECT_EQ(second->getProps(), first->getProps()); + + uiManager_->setNativeProps_DEPRECATED( + currentViewNode(), RawProps(folly::dynamic::object("opacity", 0.25))); + auto third = second->clone({}); + EXPECT_NE(third->getProps(), second->getProps()); + EXPECT_FLOAT_EQ( + static_cast(*third->getProps()).opacity, 0.25); +} + +// Another clone of a node whose props don't include the props set by +// `setNativeProps_DEPRECATED` still applies them, even after a clone of the +// same node already did. +TEST_F(FabricUIManagerTest, CloneAppliesSetNativePropsToEveryNodeWithoutThem) { + auto original = currentViewNode(); + uiManager_->setNativeProps_DEPRECATED( + original, RawProps(folly::dynamic::object("opacity", 0.5))); + + original->clone({}); + auto second = original->clone({}); + EXPECT_NE(second->getProps(), original->getProps()); + EXPECT_FLOAT_EQ( + static_cast(*second->getProps()).opacity, 0.5); +} + +// After a React re-render replaces a node's props, the next clone applies the +// props set by `setNativeProps_DEPRECATED` again. +TEST_F(FabricUIManagerTest, CloneAppliesSetNativePropsAfterReactRender) { + uiManager_->setNativeProps_DEPRECATED( + currentViewNode(), RawProps(folly::dynamic::object("opacity", 0.5))); + auto applied = currentViewNode()->clone({}); + + auto rerendered = uiManager_->cloneNode( + *applied, + /*children=*/nullptr, + RawProps(folly::dynamic::object("nativeID", "from-react"))); + auto cloned = rerendered->clone({}); + EXPECT_NE(cloned->getProps(), rerendered->getProps()); + auto& props = static_cast(*cloned->getProps()); + EXPECT_EQ(props.nativeId, "from-react"); + EXPECT_FLOAT_EQ(props.opacity, 0.5); +} + } // namespace facebook::react