Browse Source

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 <yogev132@gmail.com>
Guy Carmeli 4 years ago
parent
commit
86b344c7a2
No account linked to committer's email address

+ 7
- 0
e2e/StaticLifecycleEvents.test.js View File

@@ -60,4 +60,11 @@ describe('static lifecycle events', () => {
60 60
     await elementById(TestIDs.PUSH_BTN).tap();
61 61
     await expect(elementByLabel('componentDidDisappear | ReactTitleView | TopBarTitle')).toBeVisible();
62 62
   });
63
+
64
+  it('unmounts previous root before resolving setRoot promise', async () => {
65
+    await elementById(TestIDs.SET_ROOT_BTN).tap();
66
+    await elementById(TestIDs.SET_ROOT_BTN).tap();
67
+
68
+    await expect(elementByLabel('setRoot complete - previous root is unmounted')).toBeVisible();
69
+  });
63 70
 });

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/navigator/Navigator.java View File

@@ -144,8 +144,8 @@ public class Navigator extends ParentController {
144 144
             @Override
145 145
             public void onSuccess(String childId) {
146 146
                 if (removeSplashView) contentLayout.removeViewAt(0);
147
-                super.onSuccess(childId);
148 147
                 destroyPreviousRoot();
148
+                super.onSuccess(childId);
149 149
             }
150 150
         }, reactInstanceManager);
151 151
     }

+ 4
- 3
lib/ios/RNNCommandsHandler.m View File

@@ -71,17 +71,18 @@ static NSString* const setDefaultOptions	= @"setDefaultOptions";
71 71
 	}
72 72
 	
73 73
 	[_modalManager dismissAllModalsAnimated:NO completion:nil];
74
-	
75
-	UIViewController *vc = [_controllerFactory createLayout:layout[@"root"]];
74
+    
75
+    UIViewController *vc = [_controllerFactory createLayout:layout[@"root"]];
76 76
     vc.waitForRender = [vc.resolveOptionsWithDefault.animations.setRoot.waitForRender getWithDefaultValue:NO];
77 77
     __weak UIViewController* weakVC = vc;
78 78
     [vc setReactViewReadyCallback:^{
79
+        [self->_mainWindow.rootViewController destroy];
79 80
         self->_mainWindow.rootViewController = weakVC;
80 81
         [self->_eventEmitter sendOnNavigationCommandCompletion:setRoot commandId:commandId params:@{@"layout": layout}];
81 82
         completion();
82 83
     }];
83 84
     
84
-	[vc render];
85
+    [vc render];
85 86
 }
86 87
 
87 88
 - (void)mergeOptions:(NSString*)componentId options:(NSDictionary*)mergeOptions completion:(RNNTransitionCompletionBlock)completion {

+ 2
- 0
lib/ios/RNNComponentViewController.h View File

@@ -29,4 +29,6 @@ typedef void (^PreviewCallback)(UIViewController *vc);
29 29
 
30 30
 - (void)onButtonPress:(RNNUIBarButtonItem *)barButtonItem;
31 31
 
32
+- (void)destroyReactView;
33
+
32 34
 @end

+ 6
- 0
lib/ios/RNNComponentViewController.m View File

@@ -49,6 +49,12 @@
49 49
     [self renderReactViewIfNeeded];
50 50
 }
51 51
 
52
+- (void)destroyReactView {
53
+    if ([self.view isKindOfClass: [RNNReactView class]]) {
54
+        [((RNNReactView *)self.view) invalidate];
55
+    }
56
+}
57
+
52 58
 - (void)renderReactViewIfNeeded {
53 59
     if (!self.isViewLoaded) {
54 60
         self.view = [self.creator createRootView:self.layoutInfo.name

+ 2
- 0
lib/ios/RNNReactView.h View File

@@ -26,4 +26,6 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
26 26
 
27 27
 - (void)componentDidDisappear;
28 28
 
29
+- (void)invalidate;
30
+
29 31
 @end

+ 5
- 0
lib/ios/RNNReactView.m View File

@@ -1,4 +1,5 @@
1 1
 #import "RNNReactView.h"
2
+#import <React/RCTRootContentView.h>
2 3
 
3 4
 @implementation RNNReactView {
4 5
     BOOL _isAppeared;
@@ -45,6 +46,10 @@
45 46
     _isAppeared = NO;
46 47
 }
47 48
 
49
+- (void)invalidate {
50
+    [((RCTRootContentView *)self.contentView) invalidate];
51
+}
52
+
48 53
 - (NSString *)componentId {
49 54
 	return self.appProperties[@"componentId"];
50 55
 }

+ 2
- 0
lib/ios/UIViewController+LayoutProtocol.h View File

@@ -16,6 +16,8 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
16 16
 
17 17
 - (UIViewController *)topMostViewController;
18 18
 
19
+- (void)destroy;
20
+
19 21
 - (void)mergeOptions:(RNNNavigationOptions *)options;
20 22
 
21 23
 - (void)mergeChildOptions:(RNNNavigationOptions *)options child:(UIViewController *)child;

+ 13
- 0
lib/ios/UIViewController+LayoutProtocol.m View File

@@ -93,6 +93,19 @@
93 93
     return nil;
94 94
 }
95 95
 
96
+- (void)destroy {
97
+    [self destroyReactView];
98
+    [self.presentedViewController destroy];
99
+    
100
+    for (UIViewController* child in self.childViewControllers) {
101
+        [child destroy];
102
+    }
103
+}
104
+
105
+- (void)destroyReactView {
106
+    
107
+}
108
+
96 109
 - (UIViewController *)presentedComponentViewController {
97 110
     UIViewController* currentChild = self.getCurrentChild;
98 111
     return currentChild ? currentChild.presentedComponentViewController : self;

+ 19
- 1
playground/src/screens/SetRootScreen.js View File

@@ -4,9 +4,12 @@ const Button = require('../components/Button')
4 4
 const Navigation = require('./../services/Navigation');
5 5
 const {
6 6
   NAVIGATION_TAB,
7
-  SET_MULTIPLE_ROOTS_BTN
7
+  SET_MULTIPLE_ROOTS_BTN,
8
+  SET_ROOT_BTN
8 9
 } = require('../testIDs');
9 10
 const Screens = require('./Screens');
11
+const { logLifecycleEvent } = require('./StaticLifecycleOverlay');
12
+let unmounted;
10 13
 
11 14
 class SetRootScreen  extends React.Component {
12 15
   static options() {
@@ -24,14 +27,29 @@ class SetRootScreen  extends React.Component {
24 27
     };
25 28
   }
26 29
 
30
+  constructor(props) {
31
+    super(props);
32
+    unmounted = false;
33
+  }
34
+
27 35
   render() {
28 36
     return (
29 37
       <Root componentId={this.props.componentId}>
38
+        <Button label='Set Root' testID={SET_ROOT_BTN} onPress={this.setSingleRoot} />
30 39
         <Button label='Set Multiple Roots' testID={SET_MULTIPLE_ROOTS_BTN} onPress={this.setMultipleRoot} />
31 40
       </Root>
32 41
     );
33 42
   }
34 43
 
44
+  componentWillUnmount() {
45
+    unmounted = true;
46
+  }
47
+
48
+  setSingleRoot = async () => {
49
+    await this.setRoot();
50
+    logLifecycleEvent({text: `setRoot complete - previous root is${unmounted ? '' : ' not'} unmounted`});
51
+  }
52
+
35 53
   setMultipleRoot = async () => {
36 54
     await this.setRoot();
37 55
     await this.setRoot();

+ 5
- 1
playground/src/screens/StaticEventsScreen.js View File

@@ -6,7 +6,9 @@ const {
6 6
   PUSH_BTN,
7 7
   POP_BTN,
8 8
   STATIC_EVENTS_OVERLAY_BTN,
9
-  MODAL_BTN
9
+  MODAL_BTN,
10
+  SET_ROOT_BTN
11
+
10 12
 } = require('../testIDs');
11 13
 const Screens = require('./Screens');
12 14
 
@@ -18,6 +20,7 @@ class StaticEventsScreen extends React.Component {
18 20
         <Button label='Push' testID={PUSH_BTN} onPress={this.push} />
19 21
         <Button label='Pop' testID={POP_BTN} onPress={this.pop} />
20 22
         <Button label='Show Modal' testID={MODAL_BTN} onPress={this.showModal} />
23
+        <Button label='Set Root' testID={SET_ROOT_BTN} onPress={this.setRoot} />
21 24
       </Root>
22 25
     );
23 26
   }
@@ -37,6 +40,7 @@ class StaticEventsScreen extends React.Component {
37 40
   });
38 41
   push = () => Navigation.push(this, Screens.Pushed);
39 42
   pop = () => Navigation.pop(this);
43
+  setRoot = () => Navigation.setRoot(Screens.SetRoot);
40 44
 }
41 45
 
42 46
 module.exports = StaticEventsScreen;

+ 22
- 1
playground/src/screens/StaticLifecycleOverlay.js View File

@@ -4,6 +4,13 @@ const { View, Text, TouchableOpacity } = require('react-native');
4 4
 const { Navigation } = require('react-native-navigation');
5 5
 const TestIDs = require('../testIDs');
6 6
 
7
+let _overlayInstance;
8
+const logLifecycleEvent = (event) => {
9
+  _overlayInstance.setState({
10
+    events: [..._overlayInstance.state.events, event]
11
+  });
12
+}
13
+
7 14
 class StaticLifecycleOverlay extends Component {
8 15
   static options() {
9 16
     return {
@@ -12,6 +19,15 @@ class StaticLifecycleOverlay extends Component {
12 19
       }
13 20
     }
14 21
   }
22
+
23
+  componentDidMount() {
24
+    _overlayInstance = this;
25
+  }
26
+
27
+  componentWillUnmount() {
28
+    _overlayInstance = null;
29
+  }
30
+
15 31
   constructor(props) {
16 32
     super(props);
17 33
     this.state = {
@@ -63,6 +79,8 @@ class StaticLifecycleOverlay extends Component {
63 79
       return <Text style={styles.h2}>{`${event.event} | ${event.componentName} | ${event.componentType}`}</Text>;
64 80
     } else if (event.buttonId) {
65 81
       return <Text style={styles.h2}>{`${event.event} | ${event.buttonId}`}</Text>;
82
+    } else if (event.text){
83
+      return <Text style={styles.h2}>{`${event.text}`}</Text>;
66 84
     } else {
67 85
       return <Text style={styles.h2}>{`${event.event} | ${event.componentId}`}</Text>;
68 86
     }
@@ -109,7 +127,10 @@ class StaticLifecycleOverlay extends Component {
109 127
     );
110 128
   }
111 129
 }
112
-module.exports = StaticLifecycleOverlay;
130
+module.exports = {
131
+  StaticLifecycleOverlay,
132
+  logLifecycleEvent
133
+}
113 134
 
114 135
 const styles = {
115 136
   root: {

+ 1
- 1
playground/src/screens/index.js View File

@@ -13,7 +13,7 @@ function registerScreens() {
13 13
   Navigation.registerComponent(Screens.CocktailDetailsScreen, () => require('./sharedElementTransition/CocktailDetailsScreen'));
14 14
   Navigation.registerComponent(Screens.CocktailsListScreen, () => require('./sharedElementTransition/CocktailsListScreen'));
15 15
   Navigation.registerComponent(Screens.CocktailsListMasterScreen, () => require('./splitView/CocktailsListMasterScreen'));
16
-  Navigation.registerComponent(Screens.EventsOverlay, () => require('./StaticLifecycleOverlay'));
16
+  Navigation.registerComponent(Screens.EventsOverlay, () => require('./StaticLifecycleOverlay').StaticLifecycleOverlay);
17 17
   Navigation.registerComponent(Screens.EventsScreen, () => require('./StaticEventsScreen'));
18 18
   Navigation.registerComponent(Screens.ExternalComponent, () => require('./ExternalComponentScreen'));
19 19
   Navigation.registerComponent(Screens.FirstBottomTabsScreen, () => require('./FirstBottomTabScreen'));