Skip to content

Commit 0ab840a

Browse files
authored
WOAA-286 endless loading spinner (#8276)
* initial commit * possible fix * fixes * Fix unmount timing on iOS pop: destroy React view synchronously On programmatic pop, snapshot the outgoing screen and tear down its React view immediately so componentWillUnmount fires before the next screen mounts. This prevents shared-state races when a screen is popped and re-pushed in quick succession. For interactive pops (swipe-back), skip the early teardown so the React view stays alive if the gesture is cancelled, falling back to the existing delegate cleanup path. Also guards against no-op pops (e.g. popping root) leaving a stuck snapshot, and avoids duplicate componentDidDisappear in multi-pop flows by only emitting it for the previously visible screen.
1 parent 57dcfdc commit 0ab840a

10 files changed

Lines changed: 193 additions & 1 deletion

ios/RNNComponentViewController.mm

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,11 @@ - (void)updateSearchResultsForSearchController:(UISearchController *)searchContr
155155
isFocused:searchController.searchBar.isFirstResponder];
156156
}
157157

158+
- (void)destroyReactView {
159+
[self.reactView removeFromSuperview];
160+
self.reactView = nil;
161+
}
162+
158163
- (void)screenPopped {
159164
[_eventEmitter sendScreenPoppedEvent:self.layoutInfo.componentId];
160165
}

ios/RNNStackController.mm

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,82 @@ - (void)mergeChildOptions:(RNNNavigationOptions *)options child:(UIViewControlle
4242

4343
- (UIViewController *)popViewControllerAnimated:(BOOL)animated {
4444
[self prepareForPop];
45-
return [super popViewControllerAnimated:animated];
45+
UIViewController *previousTop = self.topViewController;
46+
UIView *snapshot = [self snapshotTopView:animated];
47+
48+
UIViewController *poppedVC = [super popViewControllerAnimated:animated];
49+
if (!poppedVC) {
50+
[snapshot removeFromSuperview];
51+
return nil;
52+
}
53+
54+
id<UIViewControllerTransitionCoordinator> coordinator = self.transitionCoordinator;
55+
if (coordinator && coordinator.isInteractive) {
56+
// Interactive pop (swipe-back): remove snapshot overlay — UIKit shows the live
57+
// view during the gesture. Skip early teardown so the React view stays alive
58+
// if the gesture is cancelled. The delegate's didShowViewController handles
59+
// cleanup once the animation finishes.
60+
[snapshot removeFromSuperview];
61+
} else {
62+
[self teardownPoppedControllers:@[ poppedVC ] previousTop:previousTop];
63+
}
64+
return poppedVC;
65+
}
66+
67+
- (NSArray<UIViewController *> *)popToViewController:(UIViewController *)viewController
68+
animated:(BOOL)animated {
69+
UIViewController *previousTop = self.topViewController;
70+
UIView *snapshot = [self snapshotTopView:animated];
71+
72+
NSArray<UIViewController *> *poppedVCs =
73+
[super popToViewController:viewController animated:animated];
74+
if (poppedVCs.count > 0) {
75+
[self teardownPoppedControllers:poppedVCs previousTop:previousTop];
76+
} else {
77+
[snapshot removeFromSuperview];
78+
}
79+
return poppedVCs;
80+
}
81+
82+
- (NSArray<UIViewController *> *)popToRootViewControllerAnimated:(BOOL)animated {
83+
UIViewController *previousTop = self.topViewController;
84+
UIView *snapshot = [self snapshotTopView:animated];
85+
86+
NSArray<UIViewController *> *poppedVCs =
87+
[super popToRootViewControllerAnimated:animated];
88+
if (poppedVCs.count > 0) {
89+
[self teardownPoppedControllers:poppedVCs previousTop:previousTop];
90+
} else {
91+
[snapshot removeFromSuperview];
92+
}
93+
return poppedVCs;
94+
}
95+
96+
#pragma mark - React view teardown
97+
98+
- (UIView *)snapshotTopView:(BOOL)animated {
99+
if (!animated) return nil;
100+
UIViewController *topVC = self.topViewController;
101+
if (!topVC.isViewLoaded || !topVC.view.window) return nil;
102+
103+
UIView *snapshot = [topVC.view snapshotViewAfterScreenUpdates:NO];
104+
if (snapshot) {
105+
snapshot.frame = topVC.view.bounds;
106+
snapshot.autoresizingMask =
107+
UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
108+
[topVC.view addSubview:snapshot];
109+
}
110+
return snapshot;
111+
}
112+
113+
- (void)teardownPoppedControllers:(NSArray<UIViewController *> *)poppedVCs
114+
previousTop:(UIViewController *)previousTop {
115+
for (UIViewController *vc in poppedVCs) {
116+
if (vc == previousTop && [vc isKindOfClass:[RNNComponentViewController class]]) {
117+
[[(RNNComponentViewController *)vc reactView] componentDidDisappear];
118+
}
119+
[vc destroyReactView];
120+
}
46121
}
47122

48123
- (BOOL)navigationBar:(UINavigationBar *)navigationBar shouldPopItem:(UINavigationItem *)item {

ios/StackControllerDelegate.mm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ - (void)navigationController:(UINavigationController *)navigationController
5656
if (_presentedViewController &&
5757
![navigationController.viewControllers containsObject:_presentedViewController]) {
5858
[_presentedViewController screenPopped];
59+
[_presentedViewController destroyReactView];
5960
_isPopping = NO;
6061
}
6162

ios/UIViewController+LayoutProtocol.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
1818

1919
- (void)destroy;
2020

21+
- (void)destroyReactView;
22+
2123
- (void)mergeOptions:(RNNNavigationOptions *)options;
2224

2325
- (void)mergeChildOptions:(RNNNavigationOptions *)options child:(UIViewController *)child;

playground/e2e/Stack.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ describe('Stack', () => {
183183
await expect(elementByLabel('pop promise resolved with: ChildId')).toBeVisible();
184184
});
185185

186+
it.e2e('pop and re-push same component should not have stale unmount', async () => {
187+
await elementById(TestIDs.PUSH_UNMOUNT_RACE_BTN).tap();
188+
await sleep(800);
189+
await expect(elementByLabel('loaded')).toBeVisible();
190+
await elementById(TestIDs.POP_AND_REPUSH_BTN).tap();
191+
await sleep(1000);
192+
await expect(elementByLabel('loaded')).toBeVisible();
193+
});
194+
186195
it('pop from root screen should do nothing', async () => {
187196
await elementById(TestIDs.POP_BTN).tap();
188197
await expect(elementById(TestIDs.STACK_SCREEN_HEADER)).toBeVisible();

playground/src/screens/Screens.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ const Screens = {
122122
},
123123
SystemUiOptions: SystemUiOptions,
124124
StatusBarFirstTab,
125+
UnmountRace: 'UnmountRace',
125126
KeyboardScreen: 'KeyboardScreen',
126127
TopBarBackground: 'TopBarBackground',
127128
Toast: 'Toast',

playground/src/screens/StackScreen.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const {
2121
SET_STACK_ROOT_WITH_ID_BTN,
2222
STACK_COMMANDS_BTN,
2323
SET_ROOT_NAVIGATION_TAB,
24+
PUSH_UNMOUNT_RACE_BTN,
2425
POP_BTN,
2526
} = testIDs;
2627

@@ -83,6 +84,11 @@ export default class StackScreen extends React.Component<NavigationProps> {
8384
testID={STACK_COMMANDS_BTN}
8485
onPress={this.pushStackCommands}
8586
/>
87+
<Button
88+
label="Push Unmount Race"
89+
testID={PUSH_UNMOUNT_RACE_BTN}
90+
onPress={this.pushUnmountRace}
91+
/>
8692
<Button label="Pop" testID={POP_BTN} onPress={this.pop} />
8793
</Root>
8894
);
@@ -166,5 +172,13 @@ export default class StackScreen extends React.Component<NavigationProps> {
166172

167173
pushStackCommands = () => Navigation.push(this, component(Screens.StackCommands));
168174

175+
pushUnmountRace = () =>
176+
Navigation.push(this, {
177+
component: {
178+
name: Screens.UnmountRace,
179+
passProps: { stackComponentId: this.props.componentId },
180+
},
181+
});
182+
169183
pop = () => Navigation.pop(this);
170184
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import React from 'react';
2+
import { Text, View, StyleSheet } from 'react-native';
3+
import { NavigationProps } from 'react-native-navigation';
4+
import Root from '../components/Root';
5+
import Button from '../components/Button';
6+
import Navigation from '../services/Navigation';
7+
import Screens from './Screens';
8+
import testIDs from '../testIDs';
9+
10+
let sharedData: string | null = null;
11+
12+
interface Props extends NavigationProps {
13+
stackComponentId?: string;
14+
}
15+
16+
export default class UnmountRaceScreen extends React.Component<Props, { data: string }> {
17+
static options() {
18+
return {
19+
topBar: {
20+
title: {
21+
text: 'Unmount Race',
22+
},
23+
},
24+
};
25+
}
26+
27+
state = { data: 'loading' };
28+
private checkTimer: ReturnType<typeof setTimeout> | null = null;
29+
30+
componentDidMount() {
31+
sharedData = 'loaded';
32+
this.checkTimer = setTimeout(() => {
33+
this.setState({ data: sharedData ?? 'stale_unmount' });
34+
}, 600);
35+
}
36+
37+
componentWillUnmount() {
38+
if (this.checkTimer) {
39+
clearTimeout(this.checkTimer);
40+
}
41+
sharedData = null;
42+
}
43+
44+
render() {
45+
return (
46+
<Root componentId={this.props.componentId}>
47+
<View style={styles.container}>
48+
<Text testID={testIDs.UNMOUNT_RACE_DATA} style={styles.text}>
49+
{this.state.data}
50+
</Text>
51+
</View>
52+
<Button
53+
label="Pop and Re-push"
54+
testID={testIDs.POP_AND_REPUSH_BTN}
55+
onPress={this.popAndRepush}
56+
/>
57+
<Button label="Pop" testID={testIDs.POP_BTN} onPress={this.pop} />
58+
</Root>
59+
);
60+
}
61+
62+
popAndRepush = () => {
63+
const stackId = this.props.stackComponentId;
64+
if (!stackId) return;
65+
Navigation.pop(this.props.componentId);
66+
Navigation.push(stackId, {
67+
component: {
68+
name: Screens.UnmountRace,
69+
passProps: { stackComponentId: stackId },
70+
},
71+
});
72+
};
73+
74+
pop = () => Navigation.pop(this);
75+
}
76+
77+
const styles = StyleSheet.create({
78+
container: { padding: 20, alignItems: 'center' },
79+
text: { fontSize: 24 },
80+
});

playground/src/screens/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ function registerScreens() {
117117
() => require('./SideMenuRightScreen').default
118118
);
119119
Navigation.registerComponent(Screens.Stack, () => require('./StackScreen').default);
120+
Navigation.registerComponent(Screens.UnmountRace, () => require('./UnmountRaceScreen').default);
120121
Navigation.registerComponent(Screens.SystemUiOptions, () => require('./SystemUiOptions').default);
121122
Navigation.registerComponent(
122123
Screens.StatusBarFirstTab,

playground/src/testIDs.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,10 @@ const testIDs = {
222222
TOGGLE_PLACEMENT_BTN: 'TOGGLE_PLACEMENT_BTN',
223223
MOUNTED_SCREENS_TEXT: 'MOUNTED_SCREENS_TEXT',
224224

225+
PUSH_UNMOUNT_RACE_BTN: 'PUSH_UNMOUNT_RACE_BTN',
226+
UNMOUNT_RACE_DATA: 'UNMOUNT_RACE_DATA',
227+
POP_AND_REPUSH_BTN: 'POP_AND_REPUSH_BTN',
228+
225229
GOTO_TOPBAR_TITLE_TEST: 'GOTO_TOPBAR_TITLE_TEST',
226230
TOPBAR_TITLE_TEXT: 'TOPBAR_TITLE_TEXT',
227231
TOPBAR_TITLE_AVATAR: 'TOPBAR_TITLE_AVATAR',

0 commit comments

Comments
 (0)