mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Fix display: contents nodes not being cloned with the wrong owner (#52530)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/52530 This PR fixes two issues with `display: contents` implementation: 1. When a node with `display: contents` set is a leaf, it won't be cloned after the initial tree is built. The added test case covers this scenario. 2. It was possible for the subtree of `display: contents` nodes not to be cloned during layout. I don't have a minimal reproduction for this one, unfortunately. It was discovered in the Expensify app: https://github.com/Expensify/App/issues/65268, along with a consistent reproduction. In that specific case, it seems to be heavily tied to `react-native-onyx`, which is a state management library. Changelog: [GENERAL][FIXED] - Fixed nodes with `display: contents` set being cloned with the wrong owner X-link: https://github.com/facebook/yoga/pull/1826 Reviewed By: adityasharat, NickGerleman Differential Revision: D78084270 Pulled By: j-piasecki fbshipit-source-id: eb81f6d7dcd1665974d07261ba693e2abea239bb
This commit is contained in:
committed by
Moti Zilberman
parent
2b13f47ff9
commit
c7cd66c64e
@@ -478,16 +478,19 @@ static void zeroOutLayoutRecursively(yoga::Node* const node) {
|
||||
}
|
||||
|
||||
static void cleanupContentsNodesRecursively(yoga::Node* const node) {
|
||||
for (auto child : node->getChildren()) {
|
||||
if (child->style().display() == Display::Contents) {
|
||||
child->getLayout() = {};
|
||||
child->setLayoutDimension(0, Dimension::Width);
|
||||
child->setLayoutDimension(0, Dimension::Height);
|
||||
child->setHasNewLayout(true);
|
||||
child->setDirty(false);
|
||||
child->cloneChildrenIfNeeded();
|
||||
if (node->hasContentsChildren()) [[unlikely]] {
|
||||
node->cloneContentsChildrenIfNeeded();
|
||||
for (auto child : node->getChildren()) {
|
||||
if (child->style().display() == Display::Contents) {
|
||||
child->getLayout() = {};
|
||||
child->setLayoutDimension(0, Dimension::Width);
|
||||
child->setLayoutDimension(0, Dimension::Height);
|
||||
child->setHasNewLayout(true);
|
||||
child->setDirty(false);
|
||||
child->cloneChildrenIfNeeded();
|
||||
|
||||
cleanupContentsNodesRecursively(child);
|
||||
cleanupContentsNodesRecursively(child);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -181,6 +181,17 @@ void Node::setDirty(bool isDirty) {
|
||||
}
|
||||
}
|
||||
|
||||
void Node::setChildren(const std::vector<Node*>& children) {
|
||||
children_ = children;
|
||||
|
||||
contentsChildrenCount_ = 0;
|
||||
for (const auto& child : children) {
|
||||
if (child->style().display() == Display::Contents) {
|
||||
contentsChildrenCount_++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
bool Node::removeChild(Node* child) {
|
||||
auto p = std::find(children_.begin(), children_.end(), child);
|
||||
if (p != children_.end()) {
|
||||
@@ -380,6 +391,23 @@ void Node::cloneChildrenIfNeeded() {
|
||||
if (child->getOwner() != this) {
|
||||
child = resolveRef(config_->cloneNode(child, this, i));
|
||||
child->setOwner(this);
|
||||
|
||||
if (child->hasContentsChildren()) [[unlikely]] {
|
||||
child->cloneContentsChildrenIfNeeded();
|
||||
}
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
|
||||
void Node::cloneContentsChildrenIfNeeded() {
|
||||
size_t i = 0;
|
||||
for (Node*& child : children_) {
|
||||
if (child->style().display() == Display::Contents &&
|
||||
child->getOwner() != this) {
|
||||
child = resolveRef(config_->cloneNode(child, this, i));
|
||||
child->setOwner(this);
|
||||
child->cloneChildrenIfNeeded();
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
|
||||
@@ -96,6 +96,10 @@ class YG_EXPORT Node : public ::YGNode {
|
||||
return config_->hasErrata(errata);
|
||||
}
|
||||
|
||||
bool hasContentsChildren() const {
|
||||
return contentsChildrenCount_ != 0;
|
||||
}
|
||||
|
||||
YGDirtiedFunc getDirtiedFunc() const {
|
||||
return dirtiedFunc_;
|
||||
}
|
||||
@@ -244,15 +248,12 @@ class YG_EXPORT Node : public ::YGNode {
|
||||
owner_ = owner;
|
||||
}
|
||||
|
||||
void setChildren(const std::vector<Node*>& children) {
|
||||
children_ = children;
|
||||
}
|
||||
|
||||
// TODO: rvalue override for setChildren
|
||||
|
||||
void setConfig(Config* config);
|
||||
|
||||
void setDirty(bool isDirty);
|
||||
void setChildren(const std::vector<Node*>& children);
|
||||
void setLayoutLastOwnerDirection(Direction direction);
|
||||
void setLayoutComputedFlexBasis(FloatOptional computedFlexBasis);
|
||||
void setLayoutComputedFlexBasisGeneration(
|
||||
@@ -286,6 +287,7 @@ class YG_EXPORT Node : public ::YGNode {
|
||||
void removeChild(size_t index);
|
||||
|
||||
void cloneChildrenIfNeeded();
|
||||
void cloneContentsChildrenIfNeeded();
|
||||
void markDirtyAndPropagate();
|
||||
float resolveFlexGrow() const;
|
||||
float resolveFlexShrink() const;
|
||||
|
||||
Reference in New Issue
Block a user