Skip to content

Conversation

heoblitz
Copy link
Collaborator

What is this PR?

Currently, FlexLayout does not guarantee that the UIView hierarchy state matches the Yoga node structure. This is because nodes are only updated during the layout process by following the UIView hierarchy.
When a child UIView is dynamically removed and markDirty() is called, it causes an error and terminates the program because it doesn't align with Yoga's logic.

In React Native's case, views that become leaves are fixed and used consistently, but FlexLayout allows any UIView to become a leaf. Therefore, we add defensive code to prevent logical errors.

Impact on Existing Users

Without this defensive code, errors were already being triggered by Yoga. In all other cases, the behavior remains identical to the previous version.

close #280

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.

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();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant