Skip to content

Commit

Permalink
fixed (iOS) Relayout not Working for Nested Inline (#41352)
Browse files Browse the repository at this point in the history
Summary:
React Native, the text inline is made versatile by design. Being managed in an alien layout logic (i.e., text paragraph), inline views work seamlessly as if in normal **flex** layout. The capacities such as animation and relayout, however, requires extra efforts on native layer.

This PR fixed one critical issue for inline, i.e., `setState()` is not working when inline contains nested views.

Closes #41348

Demo (Fixed)

https://github.com/facebook/react-native/assets/149237137/2b42d657-4024-476b-bf0c-be25ef4f8c0c

## Problem in technical:

This issue is caused by a bug in `RCTShadowView::sizeThatFitsMinimumSize()` which accidentally unlink children (of yoga nodes) with their parent (owner). More specifically, on the critical path, it
1. first **shallow** clones the current node
```
  YGNodeRef clonedYogaNode = YGNodeClone(self.yogaNode);
```
2. then calls `YGNodeCalculateLayout()` using the cloned node
3. deallocate the cloned node `YGNodeFree()`

One unseen implication of `YGNodeFree()` is to unlink all its children (because of the **shallow** clone)

```
  for (size_t i = 0; i < childCount; i++) {
    auto child = node->getChild(i);
    child->setOwner(nullptr);
  }
```

Next, let's examine,

**How nullptr of owner can cause the broken `setState()` of nested inline views**

The orphan children has two consequences:
**a**. the changes on child node (`setState()`) cannot be propagated to the parent (`YGNodeMarkDirty` -> `node->markDirtyAndPropagate();`);
**b**. `YGNodeCalculateLayout()` (`yoga::calculateLayoutImpl`) will create new children instances when orphan is detected (see below)

```
  node->cloneChildrenIfNeeded(); // line 1599 # CalculateLayout.cpp
```

Both compounded are contributing the failed `setState()`. Respectively,
**a** causes early return of `YGNodeCalculateLayout()` because parent is recognized as not *dirty*;
**b** clones a new *dirty* node which replaces the child which is supposed to be *cleaned* within `YGNodeCalculateLayout()`. And this is the *dirty* node detected by the assertion mentioned in the issue description #41348.

## The fix:

The fix introduced in this PR is to relink the children with their parent in `RCTShadowView::sizeThatFitsMinimumSize()`

## Changelog:

[IOS] [FIXED] - `setState` is not working for nested inline views in text

Pull Request resolved: #41352

Test Plan:
Test directly in rn-tester
TBD

Reviewed By: yungsters

Differential Revision: D51071338

Pulled By: NickGerleman

fbshipit-source-id: 1f3d8a3e1e03cb11577f903e43f2c2cce9e07b6e
  • Loading branch information
ehsemloh authored and facebook-github-bot committed Nov 10, 2023
1 parent 94da17e commit 9b33e75
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions packages/react-native/React/Views/RCTShadowView.m
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,13 @@ - (CGSize)sizeThatFitsMinimumSize:(CGSize)minimumSize maximumSize:(CGSize)maximu
YGNodeFree(constraintYogaNode);
YGNodeFree(clonedYogaNode);

// `setOwner()` for children unlinked by `YGNodeFree()`
int childCount = YGNodeGetChildCount(self.yogaNode);
for (int i = 0; i < childCount; i++) {
YGNodeRef child = YGNodeGetChild(self.yogaNode, i);
YGNodeSwapChild(self.yogaNode, child, i);
}

return measuredSize;
}

Expand Down

0 comments on commit 9b33e75

Please sign in to comment.