feat: dont recycle RNScreenView

FabricTestExample/App.js

@@ -94,6 +94,7 @@ import TestScreenAnimation from './src/TestScreenAnimation';
 import Test1981 from './src/Test1981';
 import Test2008 from './src/Test2008';
 import Test2028 from './src/Test2028';
+import Test2069 from './src/Test2069';
 
 enableFreeze(true);
 

FabricTestExample/src/Test2069.tsx

@@ -0,0 +1,99 @@
+import { NavigationContainer } from '@react-navigation/native';
+import {
+  createNativeStackNavigator,
+  NativeStackNavigationProp,
+} from '@react-navigation/native-stack';
+import React, { useState } from 'react';
+import { StyleSheet, Text, TouchableOpacity, View } from 'react-native';
+
+type StackParamList = {
+  Home: undefined;
+  Home1: undefined;
+  Home2: undefined;
+  Home3: undefined;
+};
+
+interface MainScreenProps {
+  navigation: NativeStackNavigationProp<StackParamList>;
+}
+
+const Home = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    <Text
+      onPress={() => {
+        navigation.navigate('Home1');
+        navigation.navigate('Home2');
+      }}>
+      This is the initial View
+    </Text>
+  </View>
+);
+
+const Home1 = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    <Text onPress={() => navigation.goBack()}>This is View 1</Text>
+  </View>
+);
+
+const Home2 = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    <Text onPress={() => navigation.goBack()}>This is View 2</Text>
+  </View>
+);
+
+const Home3 = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    {/* goBack shouldn't work here. */}
+    <Text onPress={() => navigation.goBack()}>This is View 3</Text>
+  </View>
+);
+
+const Stack = createNativeStackNavigator();
+
+const Test2069 = () => {
+  const [hasChangedState, setHasChangedState] = useState(0);
+
+  return (
+    <NavigationContainer>
+      <Stack.Navigator>
+        {hasChangedState % 3 === 0 ? (
+          <>
+            <Stack.Screen name="Home" component={Home} />
+            <Stack.Screen name="Home1" component={Home1} />
+            <Stack.Screen name="Home2" component={Home2} />
+          </>
+        ) : hasChangedState % 3 === 1 ? (
+          <>
+            <Stack.Screen name="Home2" component={Home2} />
+          </>
+        ) : (
+          <>
+            <Stack.Screen name="Home3" component={Home3} />
+          </>
+        )}
+      </Stack.Navigator>
+      <TouchableOpacity
+        style={styles.button}
+        onPress={() => setHasChangedState(old => old + 1)}>
+        <Text>Change state</Text>
+      </TouchableOpacity>
+    </NavigationContainer>
+  );
+};
+
+const styles = StyleSheet.create({
+  button: {
+    justifyContent: 'center',
+    alignItems: 'center',
+    height: 100,
+  },
+  view: {
+    alignItems: 'center',
+    backgroundColor: '#b7c4bb',
+    flex: 1,
+    justifyContent: 'center',
+    padding: 12,
+  },
+});
+
+export default Test2069;

TestsExample/App.js

@@ -95,6 +95,7 @@ import Test1844 from './src/Test1844';
 import Test1864 from './src/Test1864';
 import Test1981 from './src/Test1981';
 import Test2008 from './src/Test2008';
+import Test2069 from './src/Test2069';
 
 enableFreeze(true);
 

TestsExample/src/Test2069.tsx

@@ -0,0 +1,99 @@
+import { NavigationContainer } from '@react-navigation/native';
+import {
+  createNativeStackNavigator,
+  NativeStackNavigationProp,
+} from '@react-navigation/native-stack';
+import React, { useState } from 'react';
+import { StyleSheet, Text, TouchableOpacity, View } from 'react-native';
+
+type StackParamList = {
+  Home: undefined;
+  Home1: undefined;
+  Home2: undefined;
+  Home3: undefined;
+};
+
+interface MainScreenProps {
+  navigation: NativeStackNavigationProp<StackParamList>;
+}
+
+const Home = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    <Text
+      onPress={() => {
+        navigation.navigate('Home1');
+        navigation.navigate('Home2');
+      }}>
+      This is the initial View
+    </Text>
+  </View>
+);
+
+const Home1 = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    <Text onPress={() => navigation.goBack()}>This is View 1</Text>
+  </View>
+);
+
+const Home2 = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    <Text onPress={() => navigation.goBack()}>This is View 2</Text>
+  </View>
+);
+
+const Home3 = ({ navigation }: MainScreenProps) => (
+  <View style={styles.view}>
+    {/* goBack shouldn't work here. */}
+    <Text onPress={() => navigation.goBack()}>This is View 3</Text>
+  </View>
+);
+
+const Stack = createNativeStackNavigator();
+
+const Test2069 = () => {
+  const [hasChangedState, setHasChangedState] = useState(0);
+
+  return (
+    <NavigationContainer>
+      <Stack.Navigator>
+        {hasChangedState % 3 === 0 ? (
+          <>
+            <Stack.Screen name="Home" component={Home} />
+            <Stack.Screen name="Home1" component={Home1} />
+            <Stack.Screen name="Home2" component={Home2} />
+          </>
+        ) : hasChangedState % 3 === 1 ? (
+          <>
+            <Stack.Screen name="Home2" component={Home2} />
+          </>
+        ) : (
+          <>
+            <Stack.Screen name="Home3" component={Home3} />
+          </>
+        )}
+      </Stack.Navigator>
+      <TouchableOpacity
+        style={styles.button}
+        onPress={() => setHasChangedState(old => old + 1)}>
+        <Text>Change state</Text>
+      </TouchableOpacity>
+    </NavigationContainer>
+  );
+};
+
+const styles = StyleSheet.create({
+  button: {
+    justifyContent: 'center',
+    alignItems: 'center',
+    height: 100,
+  },
+  view: {
+    alignItems: 'center',
+    backgroundColor: '#b7c4bb',
+    flex: 1,
+    justifyContent: 'center',
+    padding: 12,
+  },
+});
+
+export default Test2069;

ios/RNSScreen.h

@@ -41,7 +41,6 @@ namespace react = facebook::react;
 - (RNSScreenView *)screenView;
 #ifdef RCT_NEW_ARCH_ENABLED
 - (void)setViewToSnapshot:(UIView *)snapshot;
-- (void)resetViewToScreen;
 - (CGFloat)calculateHeaderHeightIsModal:(BOOL)isModal;
 #endif
 

ios/RNSScreen.mm

@@ -171,12 +171,10 @@ - (void)setStackPresentation:(RNSScreenStackPresentation)stackPresentation
     _controller.presentationController.delegate = self;
   } else if (_stackPresentation != RNSScreenStackPresentationPush) {
 #ifdef RCT_NEW_ARCH_ENABLED
-    // TODO: on Fabric, same controllers can be used as modals and then recycled and used a push which would result in
-    // this error. It would be good to check if it doesn't leak in such case.
 #else
     RCTLogError(
         @"Screen presentation updated from modal to push, this may likely result in a screen object leakage. If you need to change presentation style create a new screen object instead");
-#endif
+#endif // RCT_NEW_ARCH_ENABLED
   }
   _stackPresentation = stackPresentation;
 }
@@ -298,7 +296,6 @@ - (void)notifyDismissedWithCount:(int)dismissCount
 {
 #ifdef RCT_NEW_ARCH_ENABLED
   // If screen is already unmounted then there will be no event emitter
-  // it will be cleaned in prepareForRecycle
   if (_eventEmitter != nullptr) {
     std::dynamic_pointer_cast<const react::RNSScreenEventEmitter>(_eventEmitter)
         ->onDismissed(react::RNSScreenEventEmitter::OnDismissed{.dismissCount = dismissCount});
@@ -320,7 +317,6 @@ - (void)notifyDismissCancelledWithDismissCount:(int)dismissCount
 {
 #ifdef RCT_NEW_ARCH_ENABLED
   // If screen is already unmounted then there will be no event emitter
-  // it will be cleaned in prepareForRecycle
   if (_eventEmitter != nullptr) {
     std::dynamic_pointer_cast<const react::RNSScreenEventEmitter>(_eventEmitter)
         ->onNativeDismissCancelled(
@@ -337,7 +333,6 @@ - (void)notifyWillAppear
 {
 #ifdef RCT_NEW_ARCH_ENABLED
   // If screen is already unmounted then there will be no event emitter
-  // it will be cleaned in prepareForRecycle
   if (_eventEmitter != nullptr) {
     std::dynamic_pointer_cast<const react::RNSScreenEventEmitter>(_eventEmitter)
         ->onWillAppear(react::RNSScreenEventEmitter::OnWillAppear{});
@@ -360,7 +355,6 @@ - (void)notifyWillDisappear
   }
 #ifdef RCT_NEW_ARCH_ENABLED
   // If screen is already unmounted then there will be no event emitter
-  // it will be cleaned in prepareForRecycle
   if (_eventEmitter != nullptr) {
     std::dynamic_pointer_cast<const react::RNSScreenEventEmitter>(_eventEmitter)
         ->onWillDisappear(react::RNSScreenEventEmitter::OnWillDisappear{});
@@ -376,7 +370,6 @@ - (void)notifyAppear
 {
 #ifdef RCT_NEW_ARCH_ENABLED
   // If screen is already unmounted then there will be no event emitter
-  // it will be cleaned in prepareForRecycle
   if (_eventEmitter != nullptr) {
     std::dynamic_pointer_cast<const react::RNSScreenEventEmitter>(_eventEmitter)
         ->onAppear(react::RNSScreenEventEmitter::OnAppear{});
@@ -396,7 +389,6 @@ - (void)notifyDisappear
 {
 #ifdef RCT_NEW_ARCH_ENABLED
   // If screen is already unmounted then there will be no event emitter
-  // it will be cleaned in prepareForRecycle
   if (_eventEmitter != nullptr) {
     std::dynamic_pointer_cast<const react::RNSScreenEventEmitter>(_eventEmitter)
         ->onDisappear(react::RNSScreenEventEmitter::OnDisappear{});
@@ -676,6 +668,11 @@ - (BOOL)hasHeaderConfig
   return react::concreteComponentDescriptorProvider<react::RNSScreenComponentDescriptor>();
 }
 
++ (BOOL)shouldBeRecycled
+{
+  return NO;
+}
+
 - (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
 {
   if ([childComponentView isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
@@ -697,27 +694,6 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
 
 #pragma mark - RCTComponentViewProtocol
 
-- (void)prepareForRecycle
-{
-  [super prepareForRecycle];
-  // TODO: Make sure that there is no edge case when this should be uncommented
-  // _controller=nil;
-  _dismissed = NO;
-  _state.reset();
-  _touchHandler = nil;
-
-  // We set this prop to default value here to workaround view-recycling.
-  // Let's assume the view has had _stackPresentation == <some modal stack presentation> set
-  // before below line was executed. Then, when instantiated again (with the same modal presentation)
-  // updateProps:oldProps: method would be called and setter for stack presentation would not be called.
-  // This is crucial as in that setter we register `self.controller` as a delegate
-  // (UIAdaptivePresentationControllerDelegate) to presentation controller and this leads to buggy modal behaviour as we
-  // rely on UIAdaptivePresentationControllerDelegate callbacks. Restoring the default value and then comparing against
-  // it in updateProps:oldProps: allows for setter to be called, however if there was some additional logic to execute
-  // when stackPresentation is set to "push" the setter would not be triggered.
-  _stackPresentation = RNSScreenStackPresentationPush;
-}
-
 - (void)updateProps:(react::Props::Shared const &)props oldProps:(react::Props::Shared const &)oldProps
 {
   const auto &oldScreenProps = *std::static_pointer_cast<const react::RNSScreenProps>(_props);
@@ -781,12 +757,9 @@ - (void)updateProps:(react::Props::Shared const &)props oldProps:(react::Props::
   }
 #endif // !TARGET_OS_TV
 
-  // Notice that we compare against _stackPresentation, not oldScreenProps.stackPresentation.
-  // See comment in prepareForRecycle method for explanation.
-  RNSScreenStackPresentation newStackPresentation =
-      [RNSConvert RNSScreenStackPresentationFromCppEquivalent:newScreenProps.stackPresentation];
-  if (newStackPresentation != _stackPresentation) {
-    [self setStackPresentation:newStackPresentation];
+  if (newScreenProps.stackPresentation != oldScreenProps.stackPresentation) {
+    [self
+        setStackPresentation:[RNSConvert RNSScreenStackPresentationFromCppEquivalent:newScreenProps.stackPresentation]];
   }
 
   if (newScreenProps.stackAnimation != oldScreenProps.stackAnimation) {
@@ -993,9 +966,6 @@ - (void)viewDidAppear:(BOOL)animated
 - (void)viewDidDisappear:(BOOL)animated
 {
   [super viewDidDisappear:animated];
-#ifdef RCT_NEW_ARCH_ENABLED
-  [self resetViewToScreen];
-#endif
   if (self.parentViewController == nil && self.presentingViewController == nil) {
     if (self.screenView.preventNativeDismiss) {
       // if we want to prevent the native dismiss, we do not send dismissal event,
@@ -1411,25 +1381,10 @@ - (void)hideHeaderIfNecessary
 
 - (void)setViewToSnapshot:(UIView *)snapshot
 {
-  // modals of native stack seem not to support
-  // changing their view by just setting the view
-  if (_initialView.stackPresentation != RNSScreenStackPresentationPush) {
-    UIView *superView = self.view.superview;
-    [self.view removeFromSuperview];
-    self.view = snapshot;
-    [superView addSubview:self.view];
-  } else {
-    [self.view removeFromSuperview];
-    self.view = snapshot;
-  }
-}
-
-- (void)resetViewToScreen
-{
-  if (self.view != _initialView) {
-    [self.view removeFromSuperview];
-    self.view = _initialView;
-  }
+  UIView *superView = self.view.superview;
+  [self.view removeFromSuperview];
+  self.view = snapshot;
+  [superView addSubview:self.view];
 }
 
 #else