Skip to content

Commit 0d88498

Browse files
authored
fix(iOS, Stack): Fix for reattaching Screens when swiping back quickly (#3584)
## Description This PR introduces logic to collect only active (non-dismissed) screen controllers coming from JS state. When a screen gets dismissed on the native side, its state is sent asynchronously to the JS. In scenarios where multiple screens are dismissed quickly, this can result in stale updates being sent from JS to native. These outdated updates may reference screens that are no longer part of the native view hierarchy. Since view recycling is disabled, we do not expect the `RNSScreenView` that was dismissed to be reattached; a new instance should be created when we push an update from JS. Then the new controller instance will be initialized, so these associated with detached screens won't be reused again. Therefore, we can safely filter them out from JS state on the native side. On the JS side, the state will stabilize at some point and align with native, once we receive all asynchronous events. ```objective-c @implementation RNSScreenView { ... - (void)initCommonProps { _controller = [[RNSScreen alloc] initWithView:self]; ``` To address this, we now filter out any dismissed controllers from the list received from JS. > [!NOTE] > I'm leaving the previous behavior as the default, and I'm adding `iosPreventReattachmentOfDismissedScreens` flag that can enable this fix. We're right before a very important release, and this PR is touching the very basic logic; therefore, we don't want to have this fix enabled by default for now. You can opt-in by setting `featureFlags.experiment.iosPreventReattachmentOfDismissedScreens = true`. Any feedback would be valuable here. Closes: #2559 ## Changes - Added an attribute to mark controllers that were dismissed. - Added logic for filtering out screens dismissed natively from JS state. ## Before & after - visual documentation | Before | After | | --- | --- | | <video src="https://github.com/user-attachments/assets/684a5b6d-4a1a-4da5-9bbb-3973f2cc342f" /> | <video src="https://github.com/user-attachments/assets/839debc5-7b8c-43b8-8fa6-6ec9595e708c" /> | ## Test plan Added Test 2559. ## Checklist - [x] Included code example that can be used to test this change. - [ ] Updated / created local changelog entries in relevant test files. - [x] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes
1 parent ba52d2f commit 0d88498

13 files changed

Lines changed: 193 additions & 2 deletions

File tree

FabricExample/App.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,8 @@ featureFlags.experiment.synchronousScreenUpdatesEnabled = false
55
featureFlags.experiment.synchronousHeaderConfigUpdatesEnabled = false
66
featureFlags.experiment.synchronousHeaderSubviewUpdatesEnabled = false
77
featureFlags.experiment.androidResetScreenShadowStateOnOrientationChangeEnabled = true
8+
// TODO: @t0maboro - remove ts-ignore after release
9+
// @ts-ignore - will be present since react-native-screens 4.21.0
10+
featureFlags.experiment.iosPreventReattachmentOfDismissedScreens = false
811

912
export default App;

android/src/main/java/com/swmansion/rnscreens/ScreenStackViewManager.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,13 @@ class ScreenStackViewManager :
7373
StackFinishTransitioningEvent.EVENT_NAME to mutableMapOf("registrationName" to "onFinishTransitioning"),
7474
)
7575

76+
// iosPreventReattachmentOfDismissedScreens is not available on Android,
77+
// however we must override their setters
78+
override fun setIosPreventReattachmentOfDismissedScreens(
79+
view: ScreenStack?,
80+
value: Boolean,
81+
) = Unit
82+
7683
companion object {
7784
const val REACT_CLASS = "RNSScreenStack"
7885
}

android/src/paper/java/com/facebook/react/viewmanagers/RNSScreenStackManagerDelegate.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ public RNSScreenStackManagerDelegate(U viewManager) {
2222
}
2323
@Override
2424
public void setProperty(T view, String propName, @Nullable Object value) {
25-
super.setProperty(view, propName, value);
25+
switch (propName) {
26+
case "iosPreventReattachmentOfDismissedScreens":
27+
mViewManager.setIosPreventReattachmentOfDismissedScreens(view, value == null ? false : (boolean) value);
28+
break;
29+
default:
30+
super.setProperty(view, propName, value);
31+
}
2632
}
2733
}

android/src/paper/java/com/facebook/react/viewmanagers/RNSScreenStackManagerInterface.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@
1313
import com.facebook.react.uimanager.ViewManagerWithGeneratedInterface;
1414

1515
public interface RNSScreenStackManagerInterface<T extends View> extends ViewManagerWithGeneratedInterface {
16-
// No props
16+
void setIosPreventReattachmentOfDismissedScreens(T view, boolean value);
1717
}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import * as React from 'react';
2+
import { Button, Text, View, StyleSheet } from 'react-native';
3+
import { NavigationContainer } from '@react-navigation/native';
4+
import {
5+
createNativeStackNavigator,
6+
NativeStackNavigationProp,
7+
} from '@react-navigation/native-stack';
8+
9+
type StackParamList = {
10+
ScreenOne: undefined;
11+
ScreenTwo: undefined;
12+
ScreenThree: undefined;
13+
};
14+
15+
function ScreenOne({
16+
navigation,
17+
}: {
18+
navigation: NativeStackNavigationProp<StackParamList, 'ScreenOne'>;
19+
}) {
20+
return (
21+
<View style={styles.container}>
22+
<Text style={styles.title}>ScreenOne</Text>
23+
<Button
24+
title="Go to ScreenTwo (push)"
25+
onPress={() => navigation.push('ScreenTwo')}
26+
/>
27+
</View>
28+
);
29+
}
30+
31+
function ScreenTwo({
32+
navigation,
33+
}: {
34+
navigation: NativeStackNavigationProp<StackParamList, 'ScreenTwo'>;
35+
}) {
36+
return (
37+
<View style={styles.container}>
38+
<Text style={styles.title}>ScreenTwo</Text>
39+
<Button
40+
title="Push another ScreenTwo"
41+
onPress={() => navigation.push('ScreenTwo')}
42+
/>
43+
<Button
44+
title="Push ScreenThree"
45+
onPress={() => navigation.push('ScreenThree')}
46+
/>
47+
</View>
48+
);
49+
}
50+
51+
function ScreenThree({
52+
navigation,
53+
}: {
54+
navigation: NativeStackNavigationProp<StackParamList, 'ScreenThree'>;
55+
}) {
56+
return (
57+
<View style={styles.container}>
58+
<Text style={styles.title}>ScreenThree</Text>
59+
<Button
60+
title="Push ScreenTwo"
61+
onPress={() => navigation.push('ScreenTwo')}
62+
/>
63+
<Button
64+
title="Push another ScreenThree"
65+
onPress={() => navigation.push('ScreenThree')}
66+
/>
67+
</View>
68+
);
69+
}
70+
71+
const Stack = createNativeStackNavigator<StackParamList>();
72+
73+
export default function App() {
74+
return (
75+
<NavigationContainer>
76+
<Stack.Navigator
77+
screenOptions={{
78+
fullScreenGestureEnabled: true,
79+
}}>
80+
<Stack.Screen name="ScreenOne" component={ScreenOne} />
81+
<Stack.Screen name="ScreenTwo" component={ScreenTwo} />
82+
<Stack.Screen name="ScreenThree" component={ScreenThree} />
83+
</Stack.Navigator>
84+
</NavigationContainer>
85+
);
86+
}
87+
88+
const styles = StyleSheet.create({
89+
container: {
90+
flex: 1,
91+
alignItems: 'center',
92+
justifyContent: 'center',
93+
padding: 20,
94+
},
95+
title: {
96+
fontSize: 24,
97+
textAlign: 'center',
98+
marginBottom: 20,
99+
},
100+
});

apps/src/tests/issue-tests/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ export { default as Test2522 } from './Test2522';
123123
export { default as Test2538 } from './Test2538';
124124
export { default as Test2543 } from './Test2543'; // [E2E created](iOS): issue related to iOS formSheet initial detent
125125
export { default as Test2552 } from './Test2552';
126+
export { default as Test2559 } from './Test2559';
126127
export { default as Test2560 } from './Test2560';
127128
export { default as Test2611 } from './Test2611';
128129
export { default as Test2631 } from './Test2631';

ios/RNSScreen.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ namespace react = facebook::react;
5959
- (void)setViewToSnapshot;
6060
- (CGFloat)calculateHeaderHeightIsModal:(BOOL)isModal;
6161
#endif
62+
- (BOOL)isRemovedFromParent;
6263

6364
@end
6465

ios/RNSScreen.mm

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1620,6 +1620,7 @@ @implementation RNSScreen {
16201620
int _dismissCount;
16211621
BOOL _isSwiping;
16221622
BOOL _shouldNotify;
1623+
BOOL _isRemovedFromParent;
16231624
}
16241625

16251626
#pragma mark - Common
@@ -1630,6 +1631,7 @@ - (instancetype)initWithView:(UIView *)view
16301631
self.view = view;
16311632
_fakeView = [UIView new];
16321633
_shouldNotify = YES;
1634+
_isRemovedFromParent = NO;
16331635
#ifdef RCT_NEW_ARCH_ENABLED
16341636
_initialView = (RNSScreenView *)view;
16351637
#endif
@@ -1887,6 +1889,19 @@ - (void)willMoveToParentViewController:(UIViewController *)parent
18871889
}
18881890
}
18891891

1892+
- (void)didMoveToParentViewController:(UIViewController *)parent
1893+
{
1894+
if (parent == nil) {
1895+
// Since view recycling is disabled, we can rely on a flag indicating that the controller
1896+
// has been removed from the hierarchy, as it will not be reused.
1897+
_isRemovedFromParent = YES;
1898+
} else {
1899+
_isRemovedFromParent = NO;
1900+
}
1901+
1902+
[super didMoveToParentViewController:parent];
1903+
}
1904+
18901905
- (id)findFirstResponder:(UIView *)parent
18911906
{
18921907
if (parent.isFirstResponder) {
@@ -1901,6 +1916,13 @@ - (id)findFirstResponder:(UIView *)parent
19011916
return nil;
19021917
}
19031918

1919+
// This method allows us to check whether the Screen has been dismissed;
1920+
// works reliably, because of disabled view recycling.
1921+
- (BOOL)isRemovedFromParent
1922+
{
1923+
return _isRemovedFromParent;
1924+
}
1925+
19041926
#pragma mark - transition progress related methods
19051927

19061928
- (void)setupProgressNotification

ios/RNSScreenStack.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ NS_ASSUME_NONNULL_BEGIN
4343
@property (nonatomic) BOOL customAnimation;
4444
@property (nonatomic) BOOL disableSwipeBack;
4545

46+
@property (nonatomic, readwrite) BOOL iosPreventReattachmentOfDismissedScreens;
47+
4648
#ifdef RCT_NEW_ARCH_ENABLED
4749
#else
4850
@property (nonatomic, copy) RCTDirectEventBlock onFinishTransitioning;

ios/RNSScreenStack.mm

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,16 @@ - (void)updateContainer
774774
[pushControllers addObject:screen.controller];
775775
} else {
776776
if (screen.stackPresentation == RNSScreenStackPresentationPush) {
777+
/// For synchronizing the state with JS `screen.controller.isRemovedFromParent` was added.
778+
/// We collect only active (non-dismissed) screen controllers.
779+
/// This is necessary because when a Screen is dismissed natively, its state is sent **asynchronously** to JS.
780+
/// As a result, JS might send a delayed update back to native when multiple screens are being dismissed
781+
/// quickly. Since view recycling is disabled, once we detect that a screen has been removed from the view
782+
/// hierarchy, it won't be reused. This allows us to safely filter out dismissed screens from screens coming
783+
/// from JS state via `controllers`.
784+
if (_iosPreventReattachmentOfDismissedScreens && screen.controller.isRemovedFromParent) {
785+
continue;
786+
}
777787
[pushControllers addObject:screen.controller];
778788
} else {
779789
[modalControllers addObject:screen.controller];
@@ -1363,6 +1373,20 @@ - (void)finishScreenTransition:(BOOL)canceled
13631373
#ifdef RCT_NEW_ARCH_ENABLED
13641374
#pragma mark - Fabric specific
13651375

1376+
- (void)updateProps:(const facebook::react::Props::Shared &)props
1377+
oldProps:(const facebook::react::Props::Shared &)oldProps
1378+
{
1379+
const auto &oldScreenProps = *std::static_pointer_cast<const react::RNSScreenStackProps>(_props);
1380+
const auto &newScreenProps = *std::static_pointer_cast<const react::RNSScreenStackProps>(props);
1381+
1382+
if (newScreenProps.iosPreventReattachmentOfDismissedScreens !=
1383+
oldScreenProps.iosPreventReattachmentOfDismissedScreens) {
1384+
[self setIosPreventReattachmentOfDismissedScreens:newScreenProps.iosPreventReattachmentOfDismissedScreens];
1385+
}
1386+
1387+
[super updateProps:props oldProps:oldProps];
1388+
}
1389+
13661390
- (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
13671391
{
13681392
if (![childComponentView isKindOfClass:[RNSScreenView class]]) {

0 commit comments

Comments
 (0)