Skip to content

Commit

Permalink
Cleanup the existing child node before repositioning to avoid memory …
Browse files Browse the repository at this point in the history
…leak (#2206)

Fixes #2183, During re-renders when the child nodes where being rearranged, the new nodes were inserted without removing the existing ones.

---------

Co-authored-by: William Candillon <[email protected]>
  • Loading branch information
syaau and wcandillon authored Feb 8, 2024
1 parent ed1a9e8 commit fb7ac44
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
16 changes: 16 additions & 0 deletions package/cpp/rnskia/dom/base/JsiDomNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,14 @@ class JsiDomNode : public JsiHostObject,
enqueAsynOperation([child, weakSelf = weak_from_this()]() {
auto self = weakSelf.lock();
if (self) {
// Remove the existing instance of the child, before adding it to the
// end of the list
auto existingPosition =
std::find(self->_children.begin(), self->_children.end(), child);
if (existingPosition != self->_children.end()) {
self->_children.erase(existingPosition);
}

self->_children.push_back(child);
child->setParent(self.get());
}
Expand All @@ -390,6 +398,14 @@ class JsiDomNode : public JsiHostObject,
enqueAsynOperation([child, before, weakSelf = weak_from_this()]() {
auto self = weakSelf.lock();
if (self) {
// Remove the existing instance of the child
// before adding it in the new position
auto existingPosition =
std::find(self->_children.begin(), self->_children.end(), child);
if (existingPosition != self->_children.end()) {
self->_children.erase(existingPosition);
}

auto position =
std::find(self->_children.begin(), self->_children.end(), before);
self->_children.insert(position, child);
Expand Down
1 change: 1 addition & 0 deletions package/src/dom/nodes/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export abstract class JsiNode<P> implements Node<P> {
}

addChild(child: Node<unknown>) {
this.removeChild(child);
this._children.push(child as JsiNode<unknown>);
}

Expand Down

0 comments on commit fb7ac44

Please sign in to comment.