Skip to content

Commit 91e3f77

Browse files
javachemeta-codesync[bot]
javache
authored and
meta-codesync[bot]
committed
Make removeClippedSubviews toggle-resilient (#56211)
Summary: Pull Request resolved: #56211 The `removeClippedSubviews` prop toggle path in `RCTViewComponentView` did not handle being turned off: children that were clipped (removed from superview) remained invisible, and `_reactSubviews` became stale. This diff: - Adds `_updateRemoveClippedSubviewsState` helper that ensures consistent state when toggling. On toggle-off, re-mounts all children from `_reactSubviews` in the correct order and clears the tracking array. - Makes the unmount path resilient by cleaning up `_reactSubviews` even when `removeClippedSubviews` is off. - Splits the unmount assert into two distinct messages: "not mounted" vs "mounted inside a different view". - Adds unit tests covering toggle-off re-mounting, ordering, cleanup, and unmount-after-toggle scenarios. Changelog: [iOS][Fixed] Fixes crash when changing the value of `removeClippedSubviews` Reviewed By: sbuggay Differential Revision: D97971845 fbshipit-source-id: c5d0a99e632a23613cc5e05bcf00c2985c30d5b4
1 parent bfb6870 commit 91e3f77

File tree

2 files changed

+179
-5
lines changed

2 files changed

+179
-5
lines changed

‎packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,18 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
159159
[_reactSubviews removeObjectAtIndex:index];
160160
} else {
161161
RCTAssert(
162-
childComponentView.superview == self.currentContainerView,
163-
@"Attempt to unmount a view which is mounted inside different view. (parent: %@, child: %@, index: %@)",
162+
childComponentView.superview != nil,
163+
@"Attempt to unmount a view which is not mounted. (parent: %@, child: %@, index: %@)",
164164
self,
165165
childComponentView,
166166
@(index));
167+
RCTAssert(
168+
childComponentView.superview == self.currentContainerView,
169+
@"Attempt to unmount a view which is mounted inside a different view. (parent: %@, child: %@, index: %@, existing parent: %@)",
170+
self,
171+
childComponentView,
172+
@(index),
173+
@([childComponentView.superview tag]));
167174
RCTAssert(
168175
(self.currentContainerView.subviews.count > index) &&
169176
[self.currentContainerView.subviews objectAtIndex:index] == childComponentView,
@@ -178,6 +185,30 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
178185
[childComponentView removeFromSuperview];
179186
}
180187

188+
- (void)_updateRemoveClippedSubviewsState
189+
{
190+
if (_removeClippedSubviews) {
191+
// Toggled ON: populate _reactSubviews from the current view hierarchy.
192+
// Actual clipping will happen on the next scroll event.
193+
RCTAssert(
194+
_reactSubviews.count == 0,
195+
@"_reactSubviews should be empty when toggling removeClippedSubviews on. (view: %@, count: %@)",
196+
self,
197+
@(_reactSubviews.count));
198+
if (self.currentContainerView.subviews.count > 0) {
199+
_reactSubviews = [NSMutableArray arrayWithArray:self.currentContainerView.subviews];
200+
}
201+
} else {
202+
// Toggled OFF: re-mount all children in the correct order, then clear the tracking array.
203+
// addSubview: on an already-present child moves it to the front, so iterating in order
204+
// produces the correct subview ordering.
205+
for (UIView *view in _reactSubviews) {
206+
[self.currentContainerView addSubview:view];
207+
}
208+
[_reactSubviews removeAllObjects];
209+
}
210+
}
211+
181212
- (void)updateClippedSubviewsWithClipRect:(CGRect)clipRect relativeToView:(UIView *)clipView
182213
{
183214
if (!_removeClippedSubviews) {
@@ -243,9 +274,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
243274
if (!ReactNativeFeatureFlags::enableViewCulling()) {
244275
if (oldViewProps.removeClippedSubviews != newViewProps.removeClippedSubviews) {
245276
_removeClippedSubviews = newViewProps.removeClippedSubviews;
246-
if (_removeClippedSubviews && self.currentContainerView.subviews.count > 0) {
247-
_reactSubviews = [NSMutableArray arrayWithArray:self.currentContainerView.subviews];
248-
}
277+
[self _updateRemoveClippedSubviewsState];
249278
}
250279
}
251280

‎packages/react-native/React/Tests/Mounting/RCTViewComponentViewTests.mm‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#import <React/RCTViewComponentView.h>
9+
#import <XCTest/XCTest.h>
10+
#import <react/renderer/components/view/ViewProps.h>
11+
#import <react/renderer/components/view/ViewShadowNode.h>
12+
13+
using namespace facebook::react;
14+
15+
static Props::Shared makeViewProps(bool removeClippedSubviews)
16+
{
17+
auto props = std::make_shared<ViewProps>();
18+
props->removeClippedSubviews = removeClippedSubviews;
19+
return props;
20+
}
21+
22+
@interface RCTViewComponentViewTests : XCTestCase
23+
@end
24+
25+
@implementation RCTViewComponentViewTests
26+
27+
#pragma mark - removeClippedSubviews toggle
28+
29+
- (void)testToggleRemoveClippedSubviewsOffRemountsClippedChildren
30+
{
31+
RCTViewComponentView *parent = [RCTViewComponentView new];
32+
UIView *child1 = [UIView new];
33+
child1.frame = CGRectMake(0, 0, 50, 50);
34+
UIView *child2 = [UIView new];
35+
child2.frame = CGRectMake(0, 200, 50, 50);
36+
UIView *child3 = [UIView new];
37+
child3.frame = CGRectMake(0, 400, 50, 50);
38+
39+
// Mount children normally
40+
[parent mountChildComponentView:(id)child1 index:0];
41+
[parent mountChildComponentView:(id)child2 index:1];
42+
[parent mountChildComponentView:(id)child3 index:2];
43+
44+
XCTAssertEqual(parent.subviews.count, 3u);
45+
46+
// Toggle removeClippedSubviews ON via props
47+
auto propsOn = makeViewProps(true);
48+
[parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()];
49+
50+
// Simulate clipping: remove child2 and child3 from superview (as updateClippedSubviewsWithClipRect would)
51+
[child2 removeFromSuperview];
52+
[child3 removeFromSuperview];
53+
XCTAssertEqual(parent.subviews.count, 1u);
54+
XCTAssertNil(child2.superview);
55+
XCTAssertNil(child3.superview);
56+
57+
// Toggle removeClippedSubviews OFF via props
58+
auto propsOff = makeViewProps(false);
59+
[parent updateProps:propsOff oldProps:propsOn];
60+
61+
// All children should be re-mounted
62+
XCTAssertEqual(parent.subviews.count, 3u);
63+
XCTAssertEqual(child1.superview, parent);
64+
XCTAssertEqual(child2.superview, parent);
65+
XCTAssertEqual(child3.superview, parent);
66+
}
67+
68+
- (void)testToggleRemoveClippedSubviewsOffPreservesOrder
69+
{
70+
RCTViewComponentView *parent = [RCTViewComponentView new];
71+
UIView *child1 = [UIView new];
72+
child1.frame = CGRectMake(0, 0, 50, 50);
73+
UIView *child2 = [UIView new];
74+
child2.frame = CGRectMake(0, 100, 50, 50);
75+
UIView *child3 = [UIView new];
76+
child3.frame = CGRectMake(0, 200, 50, 50);
77+
78+
[parent mountChildComponentView:(id)child1 index:0];
79+
[parent mountChildComponentView:(id)child2 index:1];
80+
[parent mountChildComponentView:(id)child3 index:2];
81+
82+
// Toggle ON and clip child1 (first child)
83+
auto propsOn = makeViewProps(true);
84+
[parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()];
85+
[child1 removeFromSuperview];
86+
XCTAssertEqual(parent.subviews.count, 2u);
87+
88+
// Toggle OFF — all children re-mounted in correct order
89+
auto propsOff = makeViewProps(false);
90+
[parent updateProps:propsOff oldProps:propsOn];
91+
92+
XCTAssertEqual(parent.subviews.count, 3u);
93+
XCTAssertEqual(parent.subviews[0], child1);
94+
XCTAssertEqual(parent.subviews[1], child2);
95+
XCTAssertEqual(parent.subviews[2], child3);
96+
}
97+
98+
- (void)testToggleRemoveClippedSubviewsOffClearsReactSubviews
99+
{
100+
RCTViewComponentView *parent = [RCTViewComponentView new];
101+
UIView *child1 = [UIView new];
102+
child1.frame = CGRectMake(0, 0, 50, 50);
103+
104+
[parent mountChildComponentView:(id)child1 index:0];
105+
106+
// Toggle ON
107+
auto propsOn = makeViewProps(true);
108+
[parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()];
109+
110+
// Toggle OFF
111+
auto propsOff = makeViewProps(false);
112+
[parent updateProps:propsOff oldProps:propsOn];
113+
114+
// _reactSubviews should be cleared
115+
NSMutableArray *reactSubviews = [parent valueForKey:@"_reactSubviews"];
116+
XCTAssertEqual(reactSubviews.count, 0u);
117+
}
118+
119+
- (void)testUnmountAfterToggleOffCleansUpReactSubviews
120+
{
121+
RCTViewComponentView *parent = [RCTViewComponentView new];
122+
UIView *child1 = [UIView new];
123+
child1.frame = CGRectMake(0, 0, 50, 50);
124+
UIView *child2 = [UIView new];
125+
child2.frame = CGRectMake(0, 100, 50, 50);
126+
127+
// Toggle ON first, then mount children
128+
auto propsOn = makeViewProps(true);
129+
[parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()];
130+
[parent mountChildComponentView:(id)child1 index:0];
131+
[parent mountChildComponentView:(id)child2 index:1];
132+
133+
// Toggle OFF — re-mounts children
134+
auto propsOff = makeViewProps(false);
135+
[parent updateProps:propsOff oldProps:propsOn];
136+
137+
XCTAssertEqual(parent.subviews.count, 2u);
138+
139+
// Unmount child2 — should succeed without assert failures
140+
[parent unmountChildComponentView:(id)child2 index:1];
141+
XCTAssertEqual(parent.subviews.count, 1u);
142+
XCTAssertNil(child2.superview);
143+
}
144+
145+
@end

0 commit comments

Comments
 (0)