Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,80 @@ ShadowNode::Unshared ShadowNode::cloneTree(
return std::const_pointer_cast<ShadowNode>(childNode);
}

ShadowNode::Unshared ShadowNode::cloneMultipleRecursive(
const ShadowNode& shadowNode,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could this just be a instance method? Alternatively make it static if it doesn't depend on instance state.

const std::unordered_set<const ShadowNodeFamily*>& familiesToUpdate,
const std::unordered_map<const ShadowNodeFamily*, int>& childrenCount,
const std::function<Unshared(
const ShadowNode& oldShadowNode,
const ShadowNodeFragment& fragment)>& callback) const {
const auto family = &shadowNode.getFamily();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use const auto * to make it explicit this isn't a copy. Even better would be to just use the ref here, and only deref it when looking it up.

auto children = shadowNode.getChildren();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Avoid copying the list

Suggested change
auto children = shadowNode.getChildren();
const auto& children = shadowNode.getChildren();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am copying that on purpose, as I want to modify it later on in the loop.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could also model this as a std::optional and only do the copy when you first need to. That also avoids the need for shouldUpdateChildren.

auto count = childrenCount.at(family);
auto shouldUpdateChildren = false;

for (int i = 0; count > 0 && i < children.size(); i++) {
const auto childFamily = &children[i]->getFamily();
if (childrenCount.contains(childFamily)) {
count--;
shouldUpdateChildren = true;
children[i] = cloneMultipleRecursive(
*children[i], familiesToUpdate, childrenCount, callback);
}
}

const ShadowNodeFragment fragment{
ShadowNodeFragment::propsPlaceholder(),
shouldUpdateChildren
? std::make_shared<ShadowNode::ListOfShared>(std::move(children))
: ShadowNodeFragment::childrenPlaceholder()};

if (familiesToUpdate.contains(family)) {
return callback(shadowNode, fragment);
}

return shadowNode.clone(fragment);
}

ShadowNode::Unshared ShadowNode::cloneMultiple(
const std::unordered_set<const ShadowNodeFamily*>& familiesToUpdate,
const std::function<Unshared(
const ShadowNode& oldShadowNode,
const ShadowNodeFragment& fragment)>& callback) const {
std::unordered_map<const ShadowNodeFamily*, int> childrenCount;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::unordered_map<const ShadowNodeFamily*, int> childrenCount;
std::unordered_map<const std::ref<const ShadowNodeFamily>, unsigned int> childrenCounts;


for (const auto& family : familiesToUpdate) {
if (childrenCount.contains(family)) {
continue;
}

childrenCount[family] = 0;

auto ancestor = family->parent_.lock();
while ((ancestor != nullptr) && ancestor != this->family_) {
auto ancestorIt = childrenCount.find(ancestor.get());
if (ancestorIt != childrenCount.end()) {
ancestorIt->second++;
break;
}
childrenCount[ancestor.get()] = 1;

ancestor = ancestor->parent_.lock();
}

if (ancestor == this->family_){
childrenCount[ancestor.get()]++;
}
}

if (childrenCount.empty()) {
return ShadowNode::Unshared{nullptr};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to be that explicit about the return type?

Suggested change
return ShadowNode::Unshared{nullptr};
return nullptr;

}

return cloneMultipleRecursive(
*this, familiesToUpdate, childrenCount, callback);
}

#pragma mark - DebugStringConvertible

#if RN_DEBUG_STRING_CONVERTIBLE
Expand Down
23 changes: 23 additions & 0 deletions packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,20 @@ class ShadowNode : public Sealable,
const std::function<Unshared(const ShadowNode& oldShadowNode)>& callback)
const;

/*
* Clones the nodes (and the subtree containing all the nodes) by
* replacing the `oldShadowNode` for every `shadowNodeFamily` from `familiesToUpdate`
* with a node that `callback` returns.
*
* Returns `nullptr` if the operation cannot be performed successfully.
*/
Unshared cloneMultiple(
const std::unordered_set<const ShadowNodeFamily*>& familiesToUpdate,
const std::function<Unshared(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's also mention what happens if callback returns nullptr?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The implementation is not really well defined if you call this method on anything that's not the RootShadowNode. Should this live on the root instead?

const ShadowNode& oldShadowNode,
const ShadowNodeFragment& fragment)>&
callback) const;

#pragma mark - Getters

ComponentName getComponentName() const;
Expand Down Expand Up @@ -252,6 +266,15 @@ class ShadowNode : public Sealable,
const ShadowNode& sourceShadowNode,
const Props::Shared& props);

Unshared cloneMultipleRecursive(
const ShadowNode& shadowNode,
const std::unordered_set<const ShadowNodeFamily*>& familiesToUpdate,
const std::unordered_map<const ShadowNodeFamily*, int>& childrenCount,
const std::function<Unshared(
const ShadowNode& oldShadowNode,
const ShadowNodeFragment& fragment)>&
callback) const;

protected:
/*
* Traits associated with the particular `ShadowNode` class and an instance of
Expand Down
Loading