Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

RobloxRenderer doesn't remove all children #217

Closed
idiomic opened this issue Jun 12, 2019 · 5 comments
Closed

RobloxRenderer doesn't remove all children #217

idiomic opened this issue Jun 12, 2019 · 5 comments

Comments

@idiomic
Copy link
Contributor

idiomic commented Jun 12, 2019

When reconciling a primitive component containing children with a element that doesn't have children then the children are not removed. If the new element has an empty list of children, the children are properly updated.

RobloxRender.updateHostNode at line 272 checks if the children are present in the new element, and if so add/removes children as needed. However, if the node had children but the new element does, it silently ignores the children.

RobloxRender.applyProp does receive the children property being set to nil but at line 112 ignores it but it a comment notes that it should be dealt with elsewhere.

@idiomic
Copy link
Contributor Author

idiomic commented Jun 12, 2019

I suspect that RobloxRender.updateHostNode should call updateVirtualNodeWithChildren with an empty children list when the old element had children but the new element does not.

@idiomic idiomic changed the title RobloxRenderer Does Not Remove All Children RobloxRenderer doesn't remove all children Jun 12, 2019
@idiomic
Copy link
Contributor Author

idiomic commented Jun 12, 2019

This may be one of those bugs whose functionality is unknowingly being relied upon in current code bases. Submitting a fix and updating current code bases may be dangerous. Would you like me to submit a PR anyways?

@AmaranthineCodices
Copy link
Contributor

Related: ea822f6, #209, #210.

Does this still occur in latest master, or is this a duplicate of #209? If it is valid against current master, please provide a repro case, as I'm not quite understanding how this is different from that issue.

@idiomic
Copy link
Contributor Author

idiomic commented Jun 12, 2019

Yup, it is a duplicate of #209

@idiomic
Copy link
Contributor Author

idiomic commented Jun 12, 2019

Duplicate of #209

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

No branches or pull requests

2 participants