-
Notifications
You must be signed in to change notification settings - Fork 237
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.
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
Conversation
| rootFlexContainer.flex.markDirty() | ||
| rootFlexContainer.flex.layout() | ||
|
|
||
| XCTAssertTrue(rootFlexContainer.subviews.isEmpty) |
There was a problem hiding this comment.
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); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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();
}
lucdion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👌
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