Skip to content

Fix error when dynamically removing views #281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix error when dynamically removing views #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
649c734
Add validation for markDirty and SetMeasureFunc
heoblitz Oct 15, 2025
4dd7004
Add RemoveViewDynamically Test
heoblitz Oct 15, 2025
f14ddee
Add validation for markDirty node
heoblitz Oct 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Sources/YogaKit/YGLayout.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,13 @@ - (void)markDirty {
// the measure function. Since we already know that this is a leaf,
// this *should* be fine. Forgive me Hack Gods.
const YGNodeRef node = self.node;
if (!YGNodeHasMeasureFunc(node)) {
if (!YGNodeHasMeasureFunc(node) && self.numberOfChildren == 0) {
YGNodeSetMeasureFunc(node, YGMeasureView);
}

YGNodeMarkDirty(node);
if (YGNodeHasMeasureFunc(node)) {
YGNodeMarkDirty(node);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered modifying the isLeaf logic in YGLayout, but decided not to proceed because there are separate properties being used, making it difficult to guarantee the existing logic.

Therefore, add defensive logic for the assertFatal conditions inside Yoga.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

void Node::setMeasureFunc(YGMeasureFunc measureFunc) {
  if (measureFunc == nullptr) {
    // TODO: t18095186 Move nodeType to opt-in function and mark appropriate
    // places in Litho
    setNodeType(NodeType::Default);
  } else {
    yoga::assertFatalWithNode(
        this,
        children_.empty(),
        "Cannot set measure function: Nodes with measure functions cannot have "
        "children.");
    // TODO: t18095186 Move nodeType to opt-in function and mark appropriate
    // places in Litho
    setNodeType(NodeType::Text);
  }

  measureFunc_ = measureFunc;
}
void YGNodeMarkDirty(const YGNodeRef nodeRef) {
  const auto node = resolveRef(nodeRef);

  yoga::assertFatalWithNode(
      node,
      node->hasMeasureFunc(),
      "Only leaf nodes with custom measure functions "
      "should manually mark themselves as dirty");

  node->markDirtyAndPropagate();
}

}

- (NSUInteger)numberOfChildren {
Expand Down
13 changes: 13 additions & 0 deletions Tests/FlexLayoutTests/FlexLayoutTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,18 @@ final class FlexLayoutTests: XCTestCase {

XCTAssertNil(weakView, "Creation of flex should not lead to retain cycle")
}


func testRemoveViewDynamically() {
let rootFlexContainer = UIView()
rootFlexContainer.flex.addItem(UIView())
rootFlexContainer.flex.define { _ in }
rootFlexContainer.flex.layout()

rootFlexContainer.subviews.forEach { $0.removeFromSuperview() }
rootFlexContainer.flex.markDirty()
rootFlexContainer.flex.layout()

XCTAssertTrue(rootFlexContainer.subviews.isEmpty)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling markDirty() on rootFlexContainer is unnecessary, but it's an area where users can easily make mistakes.

In the code before this PR, it bypasses the early return condition and causes an error.

}
}