Skip to content

Commit b7288ce

Browse files
javachecipolleschi
javache
authored and
cipolleschi
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 e2f8b5a commit b7288ce

2 files changed

Lines changed: 179 additions & 5 deletions

File tree

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

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,18 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
155155
[_reactSubviews removeObjectAtIndex:index];
156156
} else {
157157
RCTAssert(
158-
childComponentView.superview == self.currentContainerView,
159-
@"Attempt to unmount a view which is mounted inside different view. (parent: %@, child: %@, index: %@)",
158+
childComponentView.superview != nil,
159+
@"Attempt to unmount a view which is not mounted. (parent: %@, child: %@, index: %@)",
160160
self,
161161
childComponentView,
162162
@(index));
163+
RCTAssert(
164+
childComponentView.superview == self.currentContainerView,
165+
@"Attempt to unmount a view which is mounted inside a different view. (parent: %@, child: %@, index: %@, existing parent: %@)",
166+
self,
167+
childComponentView,
168+
@(index),
169+
@([childComponentView.superview tag]));
163170
RCTAssert(
164171
(self.currentContainerView.subviews.count > index) &&
165172
[self.currentContainerView.subviews objectAtIndex:index] == childComponentView,
@@ -174,6 +181,30 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
174181
[childComponentView removeFromSuperview];
175182
}
176183

184+
- (void)_updateRemoveClippedSubviewsState
185+
{
186+
if (_removeClippedSubviews) {
187+
// Toggled ON: populate _reactSubviews from the current view hierarchy.
188+
// Actual clipping will happen on the next scroll event.
189+
RCTAssert(
190+
_reactSubviews.count == 0,
191+
@"_reactSubviews should be empty when toggling removeClippedSubviews on. (view: %@, count: %@)",
192+
self,
193+
@(_reactSubviews.count));
194+
if (self.currentContainerView.subviews.count > 0) {
195+
_reactSubviews = [NSMutableArray arrayWithArray:self.currentContainerView.subviews];
196+
}
197+
} else {
198+
// Toggled OFF: re-mount all children in the correct order, then clear the tracking array.
199+
// addSubview: on an already-present child moves it to the front, so iterating in order
200+
// produces the correct subview ordering.
201+
for (UIView *view in _reactSubviews) {
202+
[self.currentContainerView addSubview:view];
203+
}
204+
[_reactSubviews removeAllObjects];
205+
}
206+
}
207+
177208
- (void)updateClippedSubviewsWithClipRect:(CGRect)clipRect relativeToView:(UIView *)clipView
178209
{
179210
if (!_removeClippedSubviews) {
@@ -239,9 +270,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
239270
if (!ReactNativeFeatureFlags::enableViewCulling()) {
240271
if (oldViewProps.removeClippedSubviews != newViewProps.removeClippedSubviews) {
241272
_removeClippedSubviews = newViewProps.removeClippedSubviews;
242-
if (_removeClippedSubviews && self.currentContainerView.subviews.count > 0) {
243-
_reactSubviews = [NSMutableArray arrayWithArray:self.currentContainerView.subviews];
244-
}
273+
[self _updateRemoveClippedSubviewsState];
245274
}
246275
}
247276

‎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)