Skip to content

Commit b034458

Browse files
authored
Avoid running side-effects in didMoveToWin after invalidation (#700)
## Description This PR resolves an issue on iOS where we'd run a side-effect code in didMoveToWindow after the view has been invalidated (that is removed from the react-native tree). As a result we'd end up getting the following crash: ``` UIViewControllerHierarchyInconsistency child view controller:<UINavigationController: 0xREDACTED should have parent view controller:<RNSScreen: 0xREDACTED but requested parent is:<UINavigationController: 0xREDACTED> ``` The problem turned out to be related to the fact we've been attempting to reattach view to a different view controller than the one it's been previously attached to. However, the code that attaches it should not be run in the first place as the view has already been removed. After investigating it turned out that layout animations sometimes cause didMoveToWindow run despite the fact the view has been removed from the hierarchy. In this PR we add an extra check to avoid running side-effects from that callback when the view is invalidated. ## Changes We change didMoveToWindow logic in both RNSScreenStack and RNSScreenContainer such that we only run side-effect when the view has not been invalidated. To check that we add a marker _invalidated that is set in invalidate callback. ## Test code and steps to reproduce The below app can be used to reproduce the crash. Before this change the app would crash after pressing the button "move" twice. After this change the app no longer crashes. ```js import React, { Component, useState } from 'react'; import { StyleSheet, Button, View, Text, LayoutAnimation, } from 'react-native'; import { Screen, ScreenStack, ScreenStackHeaderConfig, ScreenContainer, enableScreens, } from 'react-native-screens'; enableScreens(true); function Step({ move, title, color }) { return ( <View style={[styles.view, { backgroundColor: color }]}> <Button title="Move" onPress={move} /> </View> ); } function App() { const [state, setState] = useState(0); if (state === 2) return <Step move={() => setState(0)} color="green" />; return ( <ScreenStack style={styles.container} key={state < 2 ? 'one' : 'two'}> <Screen stackPresentation="containedModal" style={styles.container}> <ScreenStack style={styles.container}> <Screen style={styles.container} stackPresentation="containedModal"> <ScreenStack style={styles.container}> <Screen style={styles.container}> <ScreenStackHeaderConfig title={'One'} /> <Step move={() => setState(1)} color="pink" /> </Screen> </ScreenStack> </Screen> </ScreenStack> </Screen> {state === 1 && ( <Screen style={styles.container} stackPresentation="containedModal" stackAnimation="fade"> <ScreenStack style={styles.container}> <Screen style={styles.container}> <Step move={() => { LayoutAnimation.configureNext(LayoutAnimation.Presets.linear); setState(2); }} color="blue" /> </Screen> </ScreenStack> </Screen> )} </ScreenStack> ); } const styles = StyleSheet.create({ container: { ...StyleSheet.absoluteFillObject, }, view: { paddingTop: 100, ...StyleSheet.absoluteFillObject, }, }); export default App; ```
1 parent f3bbd48 commit b034458

File tree

2 files changed

+16
-2
lines changed

2 files changed

+16
-2
lines changed

ios/RNSScreenContainer.m

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ - (UIViewController *)findActiveChildVC
5353

5454
@implementation RNSScreenContainerView {
5555
BOOL _needUpdate;
56+
BOOL _invalidated;
5657
__weak RNSScreenContainerManager *_manager;
5758
}
5859

@@ -63,6 +64,7 @@ - (instancetype)initWithManager:(RNSScreenContainerManager *)manager
6364
_reactSubviews = [NSMutableArray new];
6465
_controller = [[RNScreensViewController alloc] init];
6566
_needUpdate = NO;
67+
_invalidated = NO;
6668
_manager = manager;
6769
[self addSubview:_controller.view];
6870
}
@@ -216,13 +218,17 @@ - (void)didUpdateReactSubviews
216218

217219
- (void)didMoveToWindow
218220
{
219-
if (self.window) {
221+
if (self.window && !_invalidated) {
222+
// We check whether the view has been invalidated before running side-effects in didMoveToWindow
223+
// This is needed because when LayoutAnimations are used it is possible for view to be re-attached
224+
// to a window despite the fact it has been removed from the React Native view hierarchy.
220225
[self reactAddControllerToClosestParent:_controller];
221226
}
222227
}
223228

224229
- (void)invalidate
225230
{
231+
_invalidated = YES;
226232
[_controller willMoveToParentViewController:nil];
227233
[_controller removeFromParentViewController];
228234
}

ios/RNSScreenStack.m

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@ @implementation RNSScreenStackView {
4545
NSMutableArray<RNSScreenView *> *_reactSubviews;
4646
__weak RNSScreenStackManager *_manager;
4747
BOOL _hasLayout;
48+
BOOL _invalidated;
4849
}
4950

5051
- (instancetype)initWithManager:(RNSScreenStackManager*)manager
5152
{
5253
if (self = [super init]) {
5354
_hasLayout = NO;
55+
_invalidated = NO;
5456
_manager = manager;
5557
_reactSubviews = [NSMutableArray new];
5658
_presentedModals = [NSMutableArray new];
@@ -189,7 +191,12 @@ - (void)didUpdateReactSubviews
189191
- (void)didMoveToWindow
190192
{
191193
[super didMoveToWindow];
192-
[self maybeAddToParentAndUpdateContainer];
194+
if (!_invalidated) {
195+
// We check whether the view has been invalidated before running side-effects in didMoveToWindow
196+
// This is needed because when LayoutAnimations are used it is possible for view to be re-attached
197+
// to a window despite the fact it has been removed from the React Native view hierarchy.
198+
[self maybeAddToParentAndUpdateContainer];
199+
}
193200
}
194201

195202
- (void)maybeAddToParentAndUpdateContainer
@@ -473,6 +480,7 @@ - (void)layoutSubviews
473480

474481
- (void)invalidate
475482
{
483+
_invalidated = YES;
476484
for (UIViewController *controller in _presentedModals) {
477485
[controller dismissViewControllerAnimated:NO completion:nil];
478486
}

0 commit comments

Comments
 (0)