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

Can a Yoga Node have multiple owners? #1543

Closed
1 task done
kennethpu opened this issue Jan 9, 2024 · 3 comments
Closed
1 task done

Can a Yoga Node have multiple owners? #1543

kennethpu opened this issue Jan 9, 2024 · 3 comments
Labels

Comments

@kennethpu
Copy link

Report

Within YGNodeInsertChild() we assert that the child we are adding cannot already have an owner. At the same time, within YGNodeRemoveChild() we have a comment indicating that "Children may be shared between parents". These two statements seem contradictory to me - is there a logical error here or am I interpreting the assert/comments incorrectly? Under what circumstances could we expect a node to wind up with multiple owners/parents?

@nicoburns
Copy link
Contributor

Under what circumstances could we expect a node to wind up with multiple owners/parents?

My understanding is that multiple owners are (or were?) being used within React Native for copy-on-write trees that share subtrees. Thus, a node should only have a single owner within the same tree (descending from a particular root node), but it may be in multiple trees. Unless you really need this functionality I would recommend enforcing that nodes only have a single owner in your own use of Yoga.

@NickGerleman
Copy link
Contributor

Yes, the public API does not allow multiple owners. RN Fabric uses concrete (private) Yoga node implementation, and ends up bypassing this check. I suspect some of these changes might have been made without as much awareness of the layering here.

I do want to find a way to get RN Fabric onto public Yoga API. There is a world where we make a revised Yoga API, to try to allow more client flexibility (in some other ways as well). Or possibly a world where we relax more of the public API constraints to make it possible, but it's not entirely clean.

@kennethpu
Copy link
Author

Got it, thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants