From 15657c6020e769fd176e174baf4c23ffc6085461 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 25 Mar 2026 04:45:41 -0700 Subject: [PATCH] Make removeClippedSubviews toggle-resilient (#56211) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../View/RCTViewComponentView.mm | 39 ++++- .../Mounting/RCTViewComponentViewTests.mm | 145 ++++++++++++++++++ 2 files changed, 179 insertions(+), 5 deletions(-) create mode 100644 packages/react-native/React/Tests/Mounting/RCTViewComponentViewTests.mm diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm index bee9270010e3..326149316a66 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm @@ -159,11 +159,18 @@ - (void)unmountChildComponentView:(UIView *)childCompo [_reactSubviews removeObjectAtIndex:index]; } else { RCTAssert( - childComponentView.superview == self.currentContainerView, - @"Attempt to unmount a view which is mounted inside different view. (parent: %@, child: %@, index: %@)", + childComponentView.superview != nil, + @"Attempt to unmount a view which is not mounted. (parent: %@, child: %@, index: %@)", self, childComponentView, @(index)); + RCTAssert( + childComponentView.superview == self.currentContainerView, + @"Attempt to unmount a view which is mounted inside a different view. (parent: %@, child: %@, index: %@, existing parent: %@)", + self, + childComponentView, + @(index), + @([childComponentView.superview tag])); RCTAssert( (self.currentContainerView.subviews.count > index) && [self.currentContainerView.subviews objectAtIndex:index] == childComponentView, @@ -178,6 +185,30 @@ - (void)unmountChildComponentView:(UIView *)childCompo [childComponentView removeFromSuperview]; } +- (void)_updateRemoveClippedSubviewsState +{ + if (_removeClippedSubviews) { + // Toggled ON: populate _reactSubviews from the current view hierarchy. + // Actual clipping will happen on the next scroll event. + RCTAssert( + _reactSubviews.count == 0, + @"_reactSubviews should be empty when toggling removeClippedSubviews on. (view: %@, count: %@)", + self, + @(_reactSubviews.count)); + if (self.currentContainerView.subviews.count > 0) { + _reactSubviews = [NSMutableArray arrayWithArray:self.currentContainerView.subviews]; + } + } else { + // Toggled OFF: re-mount all children in the correct order, then clear the tracking array. + // addSubview: on an already-present child moves it to the front, so iterating in order + // produces the correct subview ordering. + for (UIView *view in _reactSubviews) { + [self.currentContainerView addSubview:view]; + } + [_reactSubviews removeAllObjects]; + } +} + - (void)updateClippedSubviewsWithClipRect:(CGRect)clipRect relativeToView:(UIView *)clipView { if (!_removeClippedSubviews) { @@ -243,9 +274,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared & if (!ReactNativeFeatureFlags::enableViewCulling()) { if (oldViewProps.removeClippedSubviews != newViewProps.removeClippedSubviews) { _removeClippedSubviews = newViewProps.removeClippedSubviews; - if (_removeClippedSubviews && self.currentContainerView.subviews.count > 0) { - _reactSubviews = [NSMutableArray arrayWithArray:self.currentContainerView.subviews]; - } + [self _updateRemoveClippedSubviewsState]; } } diff --git a/packages/react-native/React/Tests/Mounting/RCTViewComponentViewTests.mm b/packages/react-native/React/Tests/Mounting/RCTViewComponentViewTests.mm new file mode 100644 index 000000000000..2342922928b2 --- /dev/null +++ b/packages/react-native/React/Tests/Mounting/RCTViewComponentViewTests.mm @@ -0,0 +1,145 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import +#import +#import +#import + +using namespace facebook::react; + +static Props::Shared makeViewProps(bool removeClippedSubviews) +{ + auto props = std::make_shared(); + props->removeClippedSubviews = removeClippedSubviews; + return props; +} + +@interface RCTViewComponentViewTests : XCTestCase +@end + +@implementation RCTViewComponentViewTests + +#pragma mark - removeClippedSubviews toggle + +- (void)testToggleRemoveClippedSubviewsOffRemountsClippedChildren +{ + RCTViewComponentView *parent = [RCTViewComponentView new]; + UIView *child1 = [UIView new]; + child1.frame = CGRectMake(0, 0, 50, 50); + UIView *child2 = [UIView new]; + child2.frame = CGRectMake(0, 200, 50, 50); + UIView *child3 = [UIView new]; + child3.frame = CGRectMake(0, 400, 50, 50); + + // Mount children normally + [parent mountChildComponentView:(id)child1 index:0]; + [parent mountChildComponentView:(id)child2 index:1]; + [parent mountChildComponentView:(id)child3 index:2]; + + XCTAssertEqual(parent.subviews.count, 3u); + + // Toggle removeClippedSubviews ON via props + auto propsOn = makeViewProps(true); + [parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()]; + + // Simulate clipping: remove child2 and child3 from superview (as updateClippedSubviewsWithClipRect would) + [child2 removeFromSuperview]; + [child3 removeFromSuperview]; + XCTAssertEqual(parent.subviews.count, 1u); + XCTAssertNil(child2.superview); + XCTAssertNil(child3.superview); + + // Toggle removeClippedSubviews OFF via props + auto propsOff = makeViewProps(false); + [parent updateProps:propsOff oldProps:propsOn]; + + // All children should be re-mounted + XCTAssertEqual(parent.subviews.count, 3u); + XCTAssertEqual(child1.superview, parent); + XCTAssertEqual(child2.superview, parent); + XCTAssertEqual(child3.superview, parent); +} + +- (void)testToggleRemoveClippedSubviewsOffPreservesOrder +{ + RCTViewComponentView *parent = [RCTViewComponentView new]; + UIView *child1 = [UIView new]; + child1.frame = CGRectMake(0, 0, 50, 50); + UIView *child2 = [UIView new]; + child2.frame = CGRectMake(0, 100, 50, 50); + UIView *child3 = [UIView new]; + child3.frame = CGRectMake(0, 200, 50, 50); + + [parent mountChildComponentView:(id)child1 index:0]; + [parent mountChildComponentView:(id)child2 index:1]; + [parent mountChildComponentView:(id)child3 index:2]; + + // Toggle ON and clip child1 (first child) + auto propsOn = makeViewProps(true); + [parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()]; + [child1 removeFromSuperview]; + XCTAssertEqual(parent.subviews.count, 2u); + + // Toggle OFF — all children re-mounted in correct order + auto propsOff = makeViewProps(false); + [parent updateProps:propsOff oldProps:propsOn]; + + XCTAssertEqual(parent.subviews.count, 3u); + XCTAssertEqual(parent.subviews[0], child1); + XCTAssertEqual(parent.subviews[1], child2); + XCTAssertEqual(parent.subviews[2], child3); +} + +- (void)testToggleRemoveClippedSubviewsOffClearsReactSubviews +{ + RCTViewComponentView *parent = [RCTViewComponentView new]; + UIView *child1 = [UIView new]; + child1.frame = CGRectMake(0, 0, 50, 50); + + [parent mountChildComponentView:(id)child1 index:0]; + + // Toggle ON + auto propsOn = makeViewProps(true); + [parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()]; + + // Toggle OFF + auto propsOff = makeViewProps(false); + [parent updateProps:propsOff oldProps:propsOn]; + + // _reactSubviews should be cleared + NSMutableArray *reactSubviews = [parent valueForKey:@"_reactSubviews"]; + XCTAssertEqual(reactSubviews.count, 0u); +} + +- (void)testUnmountAfterToggleOffCleansUpReactSubviews +{ + RCTViewComponentView *parent = [RCTViewComponentView new]; + UIView *child1 = [UIView new]; + child1.frame = CGRectMake(0, 0, 50, 50); + UIView *child2 = [UIView new]; + child2.frame = CGRectMake(0, 100, 50, 50); + + // Toggle ON first, then mount children + auto propsOn = makeViewProps(true); + [parent updateProps:propsOn oldProps:ViewShadowNode::defaultSharedProps()]; + [parent mountChildComponentView:(id)child1 index:0]; + [parent mountChildComponentView:(id)child2 index:1]; + + // Toggle OFF — re-mounts children + auto propsOff = makeViewProps(false); + [parent updateProps:propsOff oldProps:propsOn]; + + XCTAssertEqual(parent.subviews.count, 2u); + + // Unmount child2 — should succeed without assert failures + [parent unmountChildComponentView:(id)child2 index:1]; + XCTAssertEqual(parent.subviews.count, 1u); + XCTAssertNil(child2.superview); +} + +@end