Skip to content
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

Fix "Child already has a owner" Assert #242

Merged
merged 5 commits into from
Jan 11, 2024

Conversation

kennethpu
Copy link

@kennethpu kennethpu commented Jan 10, 2024

Upgrading to the latest version of Yoga introduced an assert in YGNodeInsertChild() that was previously removed accidentally (see relevant commit). This assert will throw an error if the node we are trying to add already has an owner. Unfortunately, we can fall into this case quite easily if we move a view between different superviews (as demonstrated in the following contrived example).

let viewA = UIView()
let viewB = UIView()
let viewC = UIView()

flex.addItem(viewA)
flex.addItem(viewB)

// Add viewC as a subview of viewA
viewA.flex.addItem(viewC)
flex.layout()

// Add viewC as a subview of viewB instead
viewB.flex.addItem(viewC)
flex.layout() // at this point we will hit the assert

To avoid triggering this assert we should check in Flex.addItem(_ view: UIView) whether view already has a superview, and if so remove the view's associated yoga node as a child from the superview's associated yoga node. This approach mirror's how UIKit handles adding subviews, and should help ensure that Yoga and UIKit's representation of the view hierarchy remains in sync

UPDATE - Restoring the changes from this old commit (c303faa) should ensure that any lingering parent references are cleaned up before we call YGNodeInsertChild()

Copy link
Member

@lucdion lucdion left a comment

Choose a reason for hiding this comment

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

Thanks for this important fix!

@lucdion lucdion merged commit f69bcdc into layoutBox:master Jan 11, 2024
1 check passed
@lucdion
Copy link
Member

lucdion commented Jan 11, 2024

Your fix has been released. Thanks for this change

@kennethpu kennethpu deleted the fix_assert branch January 23, 2024 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants