From ddf6a63d233cb43ca75c708b1d5a6059a8bfb728 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Thu, 15 Oct 2026 15:39:05 +0300 Subject: [PATCH] Don't dirty display: none nodes because of their children A display: none node is laid out with zero size whatever its children are, and Yoga never cleans the descendants of a hidden node. updateYogaChildren dirtied a hidden node whenever it was cloned with a dirty child, so a child created or changed while the node was hidden made every later commit under it re-run layout from the root. Co-Authored-By: Claude Opus 5.5 --- .../view/YogaLayoutableShadowNode.cpp | 15 ++- .../components/view/tests/ViewTest.cpp | 94 +++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index eaf72fdb2cce..fb7c6e4ffe87 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -357,8 +357,17 @@ void YogaLayoutableShadowNode::updateYogaChildren() { bool isClean = !YGNodeIsDirty(&yogaNode_) && getChildren().size() == YGNodeGetChildCount(&yogaNode_); - auto oldYogaChildren = - isClean ? yogaNode_.getChildren() : std::vector{}; + // A `display: none` node is laid out with zero size whatever its children + // are, so its children can't make its layout stale. Yoga never cleans the + // descendants of a hidden node either, so a child that was created or + // changed while the node was hidden stays dirty until the node is shown. + // Comparing children here would dirty the hidden node, and every ancestor + // up to the root, on every commit that clones it. Showing the node again + // changes its style, which dirties it in `updateYogaProps`. + bool isHidden = yogaNode_.style().display() == yoga::Display::None; + + auto oldYogaChildren = isClean && !isHidden ? yogaNode_.getChildren() + : std::vector{}; yogaNode_.setChildren({}); yogaLayoutableChildren_.clear(); @@ -371,7 +380,7 @@ void YogaLayoutableShadowNode::updateYogaChildren() { appendYogaChild(yogaLayoutableChild); adoptYogaChild(i); - if (isClean) { + if (isClean && !isHidden) { auto yogaChildIndex = yogaLayoutableChildren_.size() - 1; auto& oldYogaChildNode = *oldYogaChildren.at(yogaChildIndex); auto& newYogaChildNode = diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp index 1fa9d32e15c1..ee71635f315c 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp @@ -232,6 +232,100 @@ TEST_F(YogaDirtyFlagTest, updatingStateForScrollViewMistNotDirtyYogaNode) { static_cast(*newRootShadowNode).layoutIfNeeded()); } +TEST_F(YogaDirtyFlagTest, dirtyChildOfHiddenNodeMustNotDirtyIt) { + /* + * A `display: none` node is laid out with zero size whatever its children + * are, and Yoga never cleans the descendants of a hidden node. A child that + * changed while its parent was hidden must *not* dirty the parent, otherwise + * every commit that clones the parent re-runs layout from the root. + */ + auto& childFamily = innerShadowNode_->getChildren().at(0)->getFamily(); + + auto hiddenRootShadowNode = rootShadowNode_->cloneTree( + innerShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDisplay(yoga::Display::None); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + }); + + /* + * Hiding the node *must* dirty it. + */ + EXPECT_TRUE( + static_cast(*hiddenRootShadowNode).layoutIfNeeded()); + + /* + * Changing the layout of a child of the hidden node dirties the child, but + * not the hidden node. + */ + auto changedRootShadowNode = hiddenRootShadowNode->cloneTree( + childFamily, [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDimension( + yoga::Dimension::Width, yoga::StyleSizeLength::points(42)); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + }); + + EXPECT_FALSE( + static_cast(*changedRootShadowNode).layoutIfNeeded()); + + /* + * The child is still dirty, because the layout pass didn't visit it. A + * later change to its non-layout props must not dirty the hidden node + * either. + */ + auto newRootShadowNode = changedRootShadowNode->cloneTree( + childFamily, [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDimension( + yoga::Dimension::Width, yoga::StyleSizeLength::points(42)); + viewProps->opacity = 0.5; + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + }); + + EXPECT_FALSE( + static_cast(*newRootShadowNode).layoutIfNeeded()); +} + +TEST_F(YogaDirtyFlagTest, showingHiddenNodeMustDirtyItAndLayOutItsChildren) { + /* + * Showing a hidden node again *must* dirty it, and the children that + * changed while it was hidden must be laid out. + */ + auto& childFamily = innerShadowNode_->getChildren().at(0)->getFamily(); + + auto hiddenRootShadowNode = rootShadowNode_->cloneTree( + innerShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDisplay(yoga::Display::None); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + }); + static_cast(*hiddenRootShadowNode).layoutIfNeeded(); + + auto changedRootShadowNode = hiddenRootShadowNode->cloneTree( + childFamily, [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDimension( + yoga::Dimension::Width, yoga::StyleSizeLength::points(42)); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + }); + static_cast(*changedRootShadowNode).layoutIfNeeded(); + + auto shownRootShadowNode = changedRootShadowNode->cloneTree( + innerShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) { + return oldShadowNode.clone( + ShadowNodeFragment{.props = std::make_shared()}); + }); + + EXPECT_TRUE( + static_cast(*shownRootShadowNode).layoutIfNeeded()); + + auto child = std::static_pointer_cast( + shownRootShadowNode->getChildren().at(1)->getChildren().at(0)); + EXPECT_EQ(child->getLayoutMetrics().displayType, DisplayType::Flex); + EXPECT_EQ(child->getLayoutMetrics().frame.size.width, 42); +} + TEST_F(YogaDirtyFlagTest, clonedPropsPreserveAspectRatio) { ContextContainer contextContainer{}; PropsParserContext parserContext{-1, contextContainer};