Fix dirtied callback firing on removed children#1890
Open
YonahKarp wants to merge 1 commit intofacebook:mainfrom
Open
Fix dirtied callback firing on removed children#1890YonahKarp wants to merge 1 commit intofacebook:mainfrom
YonahKarp wants to merge 1 commit intofacebook:mainfrom
Conversation
Summary:
As per Claude:
## Root Cause
D92280506 added excludedChild->setDirty(true) to YGNodeRemoveChild and YGNodeRemoveAllChildren. The setDirty(true) call fires the dirtiedFunc_ callback, which causes a chain reaction that makes text disappear in React Native's legacy architecture.
## The Bug Chain
1. sizeThatFitsMinimumSize: (RCTShadowView:368-403) measures inline views by cloning their Yoga node via YGNodeClone. The clone inherits dirtiedFunc_ and context_ from the original.
2. After measurement, YGNodeRemoveChild (line 391) removes the clone from the temp constraint node. The new setDirty(true) fires dirtiedFunc_ on the clone.
3. The clone's dirtiedFunc_ is RCTInlineViewYogaNodeDirtied (RCTBaseTextShadowView:20-31), set when inline views are inserted into text nodes (line 59). The callback reads context_ from the clone — which points to the real inline view — gets its React superview (the text node), and calls [baseTextShadowView dirtyLayout].
4. dirtyLayout (RCTTextShadowView:58-63) calls YGNodeMarkDirty(self.yogaNode), marking the text node dirty after Yoga had already cleared the dirty flag.
5. When uiManagerWillPerformMounting runs (RCTTextShadowView:73-77), the guard if (YGNodeIsDirty(self.yogaNode)) { return; } bails out — text is
never pushed to the native view.
#### Why setLayout({}) alone was sufficient
setLayout({}) resets generationCount to 0, which forces the Yoga layout algorithm to fully recalculate the node on the next layout pass (the needToVisitNode check in CalculateLayout.cpp:2167 compares generationCount against the current generation). The dirty flag is redundant for layout correctness but was intended as a semantic indicator.
## Fix
Suppress the dirtiedFunc_ callback when marking removed children dirty. The dirty flag should still be set (for API correctness when checking YGNodeIsDirty), but the callback must not fire because the node is being detached from the tree.
Reviewed By: NickGerleman
Differential Revision: D93010183
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
As per Claude:
Root Cause
D92280506 added excludedChild->setDirty(true) to YGNodeRemoveChild and YGNodeRemoveAllChildren. The setDirty(true) call fires the dirtiedFunc_ callback, which causes a chain reaction that makes text disappear in React Native's legacy architecture.
The Bug Chain
never pushed to the native view.
Why setLayout({}) alone was sufficient
setLayout({}) resets generationCount to 0, which forces the Yoga layout algorithm to fully recalculate the node on the next layout pass (the needToVisitNode check in CalculateLayout.cpp:2167 compares generationCount against the current generation). The dirty flag is redundant for layout correctness but was intended as a semantic indicator.
Fix
Suppress the dirtiedFunc_ callback when marking removed children dirty. The dirty flag should still be set (for API correctness when checking YGNodeIsDirty), but the callback must not fire because the node is being detached from the tree.
Reviewed By: NickGerleman
Differential Revision: D93010183