-
Notifications
You must be signed in to change notification settings - Fork 25.1k
Add cloneMultiple method to the ShadowNode class
#50624
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
Changes from all commits
06dfc41
c035d85
3734f7b
d9fc6c6
23b188e
68ad9c2
47c2bf8
b8eb348
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -368,6 +368,80 @@ ShadowNode::Unshared ShadowNode::cloneTree( | |||||
| return std::const_pointer_cast<ShadowNode>(childNode); | ||||||
| } | ||||||
|
|
||||||
| ShadowNode::Unshared ShadowNode::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 { | ||||||
| const auto family = &shadowNode.getFamily(); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||||||
| auto children = shadowNode.getChildren(); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid copying the list
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||
| 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; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| 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}; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 cloneMultipleRecursive( | ||||||
| *this, familiesToUpdate, childrenCount, callback); | ||||||
| } | ||||||
|
|
||||||
| #pragma mark - DebugStringConvertible | ||||||
|
|
||||||
| #if RN_DEBUG_STRING_CONVERTIBLE | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's also mention what happens if
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
|
@@ -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 | ||
|
|
||
There was a problem hiding this comment.
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.