Unmount previous root before resolving setRoot promise (#6197)

* Unmount previous root before resolving setRoot promise * Unmount hirerchy on setRoot * Fix tests Co-authored-by: yogevbd <[email protected]>
wix · May 17, 2020 · 86b344c · 86b344c
1 parent 3e9a2f2
commit 86b344c
Show file tree

Hide file tree

Showing 13 changed files with 89 additions and 8 deletions.
diff --git a/e2e/StaticLifecycleEvents.test.js b/e2e/StaticLifecycleEvents.test.js
@@ -60,4 +60,11 @@ describe('static lifecycle events', () => {
     await elementById(TestIDs.PUSH_BTN).tap();
     await expect(elementByLabel('componentDidDisappear | ReactTitleView | TopBarTitle')).toBeVisible();
   });
+
+  it('unmounts previous root before resolving setRoot promise', async () => {
+    await elementById(TestIDs.SET_ROOT_BTN).tap();
+    await elementById(TestIDs.SET_ROOT_BTN).tap();
+
+    await expect(elementByLabel('setRoot complete - previous root is unmounted')).toBeVisible();
+  });
 });
diff --git a/...roid/app/src/main/java/com/reactnativenavigation/viewcontrollers/navigator/Navigator.java b/...roid/app/src/main/java/com/reactnativenavigation/viewcontrollers/navigator/Navigator.java
@@ -144,8 +144,8 @@ public void setRoot(final ViewController viewController, CommandListener command
             @Override
             public void onSuccess(String childId) {
                 if (removeSplashView) contentLayout.removeViewAt(0);
-                super.onSuccess(childId);
                 destroyPreviousRoot();
+                super.onSuccess(childId);
             }
         }, reactInstanceManager);
     }

diff --git a/lib/ios/RNNCommandsHandler.m b/lib/ios/RNNCommandsHandler.m
@@ -71,17 +71,18 @@ - (void)setRoot:(NSDictionary*)layout commandId:(NSString*)commandId completion:
 	}
 
 	[_modalManager dismissAllModalsAnimated:NO completion:nil];
-	
-	UIViewController *vc = [_controllerFactory createLayout:layout[@"root"]];
+    
+    UIViewController *vc = [_controllerFactory createLayout:layout[@"root"]];
     vc.waitForRender = [vc.resolveOptionsWithDefault.animations.setRoot.waitForRender getWithDefaultValue:NO];
     __weak UIViewController* weakVC = vc;
     [vc setReactViewReadyCallback:^{
+        [self->_mainWindow.rootViewController destroy];
         self->_mainWindow.rootViewController = weakVC;
         [self->_eventEmitter sendOnNavigationCommandCompletion:setRoot commandId:commandId params:@{@"layout": layout}];
         completion();
     }];
 
-	[vc render];
+    [vc render];
 }
 
 - (void)mergeOptions:(NSString*)componentId options:(NSDictionary*)mergeOptions completion:(RNNTransitionCompletionBlock)completion {

diff --git a/lib/ios/RNNComponentViewController.h b/lib/ios/RNNComponentViewController.h
@@ -29,4 +29,6 @@ typedef void (^PreviewCallback)(UIViewController *vc);
 
 - (void)onButtonPress:(RNNUIBarButtonItem *)barButtonItem;
 
+- (void)destroyReactView;
+
 @end
diff --git a/lib/ios/RNNComponentViewController.m b/lib/ios/RNNComponentViewController.m
@@ -49,6 +49,12 @@ - (void)render {
     [self renderReactViewIfNeeded];
 }
 
+- (void)destroyReactView {
+    if ([self.view isKindOfClass: [RNNReactView class]]) {
+        [((RNNReactView *)self.view) invalidate];
+    }
+}
+
 - (void)renderReactViewIfNeeded {
     if (!self.isViewLoaded) {
         self.view = [self.creator createRootView:self.layoutInfo.name

diff --git a/lib/ios/RNNReactView.h b/lib/ios/RNNReactView.h
@@ -26,4 +26,6 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
 
 - (void)componentDidDisappear;
 
+- (void)invalidate;
+
 @end
diff --git a/lib/ios/RNNReactView.m b/lib/ios/RNNReactView.m
@@ -1,4 +1,5 @@
 #import "RNNReactView.h"
+#import <React/RCTRootContentView.h>
 
 @implementation RNNReactView {
     BOOL _isAppeared;
@@ -45,6 +46,10 @@ - (void)componentDidDisappear {
     _isAppeared = NO;
 }
 
+- (void)invalidate {
+    [((RCTRootContentView *)self.contentView) invalidate];
+}
+
 - (NSString *)componentId {
 	return self.appProperties[@"componentId"];
 }

diff --git a/lib/ios/UIViewController+LayoutProtocol.h b/lib/ios/UIViewController+LayoutProtocol.h
@@ -16,6 +16,8 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
 
 - (UIViewController *)topMostViewController;
 
+- (void)destroy;
+
 - (void)mergeOptions:(RNNNavigationOptions *)options;
 
 - (void)mergeChildOptions:(RNNNavigationOptions *)options child:(UIViewController *)child;

diff --git a/lib/ios/UIViewController+LayoutProtocol.m b/lib/ios/UIViewController+LayoutProtocol.m
@@ -93,6 +93,19 @@ - (UIViewController *)getCurrentChild {
     return nil;
 }
 
+- (void)destroy {
+    [self destroyReactView];
+    [self.presentedViewController destroy];
+
+    for (UIViewController* child in self.childViewControllers) {
+        [child destroy];
+    }
+}
+
+- (void)destroyReactView {
+
+}
+
 - (UIViewController *)presentedComponentViewController {
     UIViewController* currentChild = self.getCurrentChild;
     return currentChild ? currentChild.presentedComponentViewController : self;

diff --git a/playground/src/screens/SetRootScreen.js b/playground/src/screens/SetRootScreen.js
@@ -4,9 +4,12 @@ const Button = require('../components/Button')
 const Navigation = require('./../services/Navigation');
 const {
   NAVIGATION_TAB,
-  SET_MULTIPLE_ROOTS_BTN
+  SET_MULTIPLE_ROOTS_BTN,
+  SET_ROOT_BTN
 } = require('../testIDs');
 const Screens = require('./Screens');
+const { logLifecycleEvent } = require('./StaticLifecycleOverlay');
+let unmounted;
 
 class SetRootScreen  extends React.Component {
   static options() {
@@ -24,14 +27,29 @@ class SetRootScreen  extends React.Component {
     };
   }
 
+  constructor(props) {
+    super(props);
+    unmounted = false;
+  }
+
   render() {
     return (
       <Root componentId={this.props.componentId}>
+        <Button label='Set Root' testID={SET_ROOT_BTN} onPress={this.setSingleRoot} />
         <Button label='Set Multiple Roots' testID={SET_MULTIPLE_ROOTS_BTN} onPress={this.setMultipleRoot} />
       </Root>
     );
   }
 
+  componentWillUnmount() {
+    unmounted = true;
+  }
+
+  setSingleRoot = async () => {
+    await this.setRoot();
+    logLifecycleEvent({text: `setRoot complete - previous root is${unmounted ? '' : ' not'} unmounted`});
+  }
+
   setMultipleRoot = async () => {
     await this.setRoot();
     await this.setRoot();

diff --git a/playground/src/screens/StaticEventsScreen.js b/playground/src/screens/StaticEventsScreen.js
@@ -6,7 +6,9 @@ const {
   PUSH_BTN,
   POP_BTN,
   STATIC_EVENTS_OVERLAY_BTN,
-  MODAL_BTN
+  MODAL_BTN,
+  SET_ROOT_BTN
+
 } = require('../testIDs');
 const Screens = require('./Screens');
 
@@ -18,6 +20,7 @@ class StaticEventsScreen extends React.Component {
         <Button label='Push' testID={PUSH_BTN} onPress={this.push} />
         <Button label='Pop' testID={POP_BTN} onPress={this.pop} />
         <Button label='Show Modal' testID={MODAL_BTN} onPress={this.showModal} />
+        <Button label='Set Root' testID={SET_ROOT_BTN} onPress={this.setRoot} />
       </Root>
     );
   }
@@ -37,6 +40,7 @@ class StaticEventsScreen extends React.Component {
   });
   push = () => Navigation.push(this, Screens.Pushed);
   pop = () => Navigation.pop(this);
+  setRoot = () => Navigation.setRoot(Screens.SetRoot);
 }
 
 module.exports = StaticEventsScreen;
diff --git a/playground/src/screens/StaticLifecycleOverlay.js b/playground/src/screens/StaticLifecycleOverlay.js
@@ -4,6 +4,13 @@ const { View, Text, TouchableOpacity } = require('react-native');
 const { Navigation } = require('react-native-navigation');
 const TestIDs = require('../testIDs');
 
+let _overlayInstance;
+const logLifecycleEvent = (event) => {
+  _overlayInstance.setState({
+    events: [..._overlayInstance.state.events, event]
+  });
+}
+
 class StaticLifecycleOverlay extends Component {
   static options() {
     return {
@@ -12,6 +19,15 @@ class StaticLifecycleOverlay extends Component {
       }
     }
   }
+
+  componentDidMount() {
+    _overlayInstance = this;
+  }
+
+  componentWillUnmount() {
+    _overlayInstance = null;
+  }
+
   constructor(props) {
     super(props);
     this.state = {
@@ -63,6 +79,8 @@ class StaticLifecycleOverlay extends Component {
       return <Text style={styles.h2}>{`${event.event} | ${event.componentName} | ${event.componentType}`}</Text>;
     } else if (event.buttonId) {
       return <Text style={styles.h2}>{`${event.event} | ${event.buttonId}`}</Text>;
+    } else if (event.text){
+      return <Text style={styles.h2}>{`${event.text}`}</Text>;
     } else {
       return <Text style={styles.h2}>{`${event.event} | ${event.componentId}`}</Text>;
     }
@@ -109,7 +127,10 @@ class StaticLifecycleOverlay extends Component {
     );
   }
 }
-module.exports = StaticLifecycleOverlay;
+module.exports = {
+  StaticLifecycleOverlay,
+  logLifecycleEvent
+}
 
 const styles = {
   root: {

diff --git a/playground/src/screens/index.js b/playground/src/screens/index.js
@@ -13,7 +13,7 @@ function registerScreens() {
   Navigation.registerComponent(Screens.CocktailDetailsScreen, () => require('./sharedElementTransition/CocktailDetailsScreen'));
   Navigation.registerComponent(Screens.CocktailsListScreen, () => require('./sharedElementTransition/CocktailsListScreen'));
   Navigation.registerComponent(Screens.CocktailsListMasterScreen, () => require('./splitView/CocktailsListMasterScreen'));
-  Navigation.registerComponent(Screens.EventsOverlay, () => require('./StaticLifecycleOverlay'));
+  Navigation.registerComponent(Screens.EventsOverlay, () => require('./StaticLifecycleOverlay').StaticLifecycleOverlay);
   Navigation.registerComponent(Screens.EventsScreen, () => require('./StaticEventsScreen'));
   Navigation.registerComponent(Screens.ExternalComponent, () => require('./ExternalComponentScreen'));
   Navigation.registerComponent(Screens.FirstBottomTabsScreen, () => require('./FirstBottomTabScreen'));