Skip to content

Commit 3fec4bf

Browse files
j-piaseckikikoso
authored andcommitted
Fix display: contents nodes not being cloned with the wrong owner (facebook#52530)
Summary: Pull Request resolved: facebook#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: Expensify/App#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: facebook/yoga#1826 Reviewed By: adityasharat, NickGerleman Differential Revision: D78084270 Pulled By: j-piasecki fbshipit-source-id: eb81f6d7dcd1665974d07261ba693e2abea239bb
1 parent 6c571e2 commit 3fec4bf

File tree

3 files changed

+47
-14
lines changed

3 files changed

+47
-14
lines changed

packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -478,16 +478,19 @@ static void zeroOutLayoutRecursively(yoga::Node* const node) {
478478
}
479479

480480
static void cleanupContentsNodesRecursively(yoga::Node* const node) {
481-
for (auto child : node->getChildren()) {
482-
if (child->style().display() == Display::Contents) {
483-
child->getLayout() = {};
484-
child->setLayoutDimension(0, Dimension::Width);
485-
child->setLayoutDimension(0, Dimension::Height);
486-
child->setHasNewLayout(true);
487-
child->setDirty(false);
488-
child->cloneChildrenIfNeeded();
489-
490-
cleanupContentsNodesRecursively(child);
481+
if (node->hasContentsChildren()) [[unlikely]] {
482+
node->cloneContentsChildrenIfNeeded();
483+
for (auto child : node->getChildren()) {
484+
if (child->style().display() == Display::Contents) {
485+
child->getLayout() = {};
486+
child->setLayoutDimension(0, Dimension::Width);
487+
child->setLayoutDimension(0, Dimension::Height);
488+
child->setHasNewLayout(true);
489+
child->setDirty(false);
490+
child->cloneChildrenIfNeeded();
491+
492+
cleanupContentsNodesRecursively(child);
493+
}
491494
}
492495
}
493496
}

packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,17 @@ void Node::setDirty(bool isDirty) {
181181
}
182182
}
183183

184+
void Node::setChildren(const std::vector<Node*>& children) {
185+
children_ = children;
186+
187+
contentsChildrenCount_ = 0;
188+
for (const auto& child : children) {
189+
if (child->style().display() == Display::Contents) {
190+
contentsChildrenCount_++;
191+
}
192+
}
193+
}
194+
184195
bool Node::removeChild(Node* child) {
185196
auto p = std::find(children_.begin(), children_.end(), child);
186197
if (p != children_.end()) {
@@ -380,6 +391,23 @@ void Node::cloneChildrenIfNeeded() {
380391
if (child->getOwner() != this) {
381392
child = resolveRef(config_->cloneNode(child, this, i));
382393
child->setOwner(this);
394+
395+
if (child->hasContentsChildren()) [[unlikely]] {
396+
child->cloneContentsChildrenIfNeeded();
397+
}
398+
}
399+
i += 1;
400+
}
401+
}
402+
403+
void Node::cloneContentsChildrenIfNeeded() {
404+
size_t i = 0;
405+
for (Node*& child : children_) {
406+
if (child->style().display() == Display::Contents &&
407+
child->getOwner() != this) {
408+
child = resolveRef(config_->cloneNode(child, this, i));
409+
child->setOwner(this);
410+
child->cloneChildrenIfNeeded();
383411
}
384412
i += 1;
385413
}

packages/react-native/ReactCommon/yoga/yoga/node/Node.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ class YG_EXPORT Node : public ::YGNode {
9696
return config_->hasErrata(errata);
9797
}
9898

99+
bool hasContentsChildren() const {
100+
return contentsChildrenCount_ != 0;
101+
}
102+
99103
YGDirtiedFunc getDirtiedFunc() const {
100104
return dirtiedFunc_;
101105
}
@@ -244,15 +248,12 @@ class YG_EXPORT Node : public ::YGNode {
244248
owner_ = owner;
245249
}
246250

247-
void setChildren(const std::vector<Node*>& children) {
248-
children_ = children;
249-
}
250-
251251
// TODO: rvalue override for setChildren
252252

253253
void setConfig(Config* config);
254254

255255
void setDirty(bool isDirty);
256+
void setChildren(const std::vector<Node*>& children);
256257
void setLayoutLastOwnerDirection(Direction direction);
257258
void setLayoutComputedFlexBasis(FloatOptional computedFlexBasis);
258259
void setLayoutComputedFlexBasisGeneration(
@@ -286,6 +287,7 @@ class YG_EXPORT Node : public ::YGNode {
286287
void removeChild(size_t index);
287288

288289
void cloneChildrenIfNeeded();
290+
void cloneContentsChildrenIfNeeded();
289291
void markDirtyAndPropagate();
290292
float resolveFlexGrow() const;
291293
float resolveFlexShrink() const;

0 commit comments

Comments
 (0)