From 2e3da407e230af7c7eeeb2b7d204677de9c871aa Mon Sep 17 00:00:00 2001 From: Yonah Karp Date: Tue, 10 Feb 2026 08:36:51 -0800 Subject: [PATCH] Yoga | dirty YGNode when removing it from parent (#1868) Summary: X-link: https://github.com/facebook/react-native/pull/55411 ## Context When a child node is removed from its parent via YGNodeRemoveChild, the child's calculated layout is cleared (setLayout({})) but the child node is not marked dirty. This causes issues when the view is removed views and later reused with the same layout values. the layout values were cleared (resulting in NaN/0), but since the layout values are the same for the newly reattached node, it isn't dirtied. However, the user would expect it to be dirty, as: a) a fully new node would be dirty by default. b) They just set layout **Example**: - Node is removed from the view and it's layout is cleared. - Node is reattatched and set with height/width that happens to be same as previous - Checking if the view is dirty unexpectedly gives false, despite user setting a "new" value. - Pulling height/width gives 0, as layout hasn't been recalculated yet ## This Diff Marks the removed child node as dirty when clearing its layout in YGNodeRemoveChild, ensuring that subsequent layout calculations will properly recalculate the child's layout values Reviewed By: NickGerleman Differential Revision: D92280506 --- tests/YGDirtyMarkingTest.cpp | 55 ++++++++++++++++++++++++++++++++++++ tests/YGNodeChildTest.cpp | 33 ++++++++++++++++++++++ yoga/YGNode.cpp | 2 ++ 3 files changed, 90 insertions(+) diff --git a/tests/YGDirtyMarkingTest.cpp b/tests/YGDirtyMarkingTest.cpp index b7da4fcaae..5a50b8deae 100644 --- a/tests/YGDirtyMarkingTest.cpp +++ b/tests/YGDirtyMarkingTest.cpp @@ -249,3 +249,58 @@ TEST(YogaTest, dirty_node_only_if_undefined_values_gets_set_to_undefined) { YGNodeFreeRecursive(root); } + +TEST(YogaTest, dirty_removed_child_node) { + YGNodeRef root = YGNodeNew(); + YGNodeStyleSetWidth(root, 100); + YGNodeStyleSetHeight(root, 100); + + YGNodeRef child = YGNodeNew(); + YGNodeStyleSetWidth(child, 50); + YGNodeStyleSetHeight(child, 50); + YGNodeInsertChild(root, child, 0); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + EXPECT_FALSE(YGNodeIsDirty(child)); + + YGNodeRemoveChild(root, child); + + // Child should be marked dirty after removal so layout is recalculated + // when the child is reused (e.g., in a recycling view system) + EXPECT_TRUE(YGNodeIsDirty(child)); + + YGNodeFree(child); + YGNodeFreeRecursive(root); +} + +TEST(YogaTest, dirty_removed_child_nodes_when_removing_all) { + YGNodeRef root = YGNodeNew(); + YGNodeStyleSetWidth(root, 100); + YGNodeStyleSetHeight(root, 100); + + YGNodeRef child0 = YGNodeNew(); + YGNodeStyleSetWidth(child0, 50); + YGNodeStyleSetHeight(child0, 25); + YGNodeInsertChild(root, child0, 0); + + YGNodeRef child1 = YGNodeNew(); + YGNodeStyleSetWidth(child1, 50); + YGNodeStyleSetHeight(child1, 25); + YGNodeInsertChild(root, child1, 1); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + EXPECT_FALSE(YGNodeIsDirty(child0)); + EXPECT_FALSE(YGNodeIsDirty(child1)); + + YGNodeRemoveAllChildren(root); + + // All children should be marked dirty after removal + EXPECT_TRUE(YGNodeIsDirty(child0)); + EXPECT_TRUE(YGNodeIsDirty(child1)); + + YGNodeFree(child0); + YGNodeFree(child1); + YGNodeFreeRecursive(root); +} diff --git a/tests/YGNodeChildTest.cpp b/tests/YGNodeChildTest.cpp index b61c1322fe..0b7148dd52 100644 --- a/tests/YGNodeChildTest.cpp +++ b/tests/YGNodeChildTest.cpp @@ -33,3 +33,36 @@ TEST(YogaTest, reset_layout_when_child_removed) { YGNodeFreeRecursive(root); YGNodeFreeRecursive(root_child0); } + +TEST(YogaTest, removed_child_can_be_reused_with_valid_layout) { + YGNodeRef root = YGNodeNew(); + YGNodeStyleSetWidth(root, 200); + YGNodeStyleSetHeight(root, 200); + + YGNodeRef child = YGNodeNew(); + YGNodeStyleSetWidth(child, 100); + YGNodeStyleSetHeight(child, 100); + YGNodeInsertChild(root, child, 0); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + ASSERT_EQ(100, YGNodeLayoutGetWidth(child)); + ASSERT_EQ(100, YGNodeLayoutGetHeight(child)); + + // Remove child - layout should be cleared and child marked dirty + YGNodeRemoveChild(root, child); + + ASSERT_TRUE(YGFloatIsUndefined(YGNodeLayoutGetWidth(child))); + ASSERT_TRUE(YGFloatIsUndefined(YGNodeLayoutGetHeight(child))); + ASSERT_TRUE(YGNodeIsDirty(child)); + + // Reinsert the child and recalculate - layout should be valid again + YGNodeInsertChild(root, child, 0); + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + ASSERT_EQ(100, YGNodeLayoutGetWidth(child)); + ASSERT_EQ(100, YGNodeLayoutGetHeight(child)); + ASSERT_FALSE(YGNodeIsDirty(child)); + + YGNodeFreeRecursive(root); +} diff --git a/yoga/YGNode.cpp b/yoga/YGNode.cpp index a34685eee6..9903478391 100644 --- a/yoga/YGNode.cpp +++ b/yoga/YGNode.cpp @@ -177,6 +177,7 @@ void YGNodeRemoveChild( if (owner == childOwner) { excludedChild->setLayout({}); // layout is no longer valid excludedChild->setOwner(nullptr); + excludedChild->setDirty(true); // invalidate cache } owner->markDirtyAndPropagate(); } @@ -198,6 +199,7 @@ void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) { yoga::Node* oldChild = owner->getChild(i); oldChild->setLayout({}); // layout is no longer valid oldChild->setOwner(nullptr); + oldChild->setDirty(true); // invalidate cache } owner->clearChildren(); owner->markDirtyAndPropagate();