NativeScript / nativescript-angular

Integrating NativeScript with Angular
http://docs.nativescript.org/angular/tutorial/ng-chapter-0
Apache License 2.0
1.21k stars 241 forks source link

Occasional crash in NSRouteReuseStrategy.shouldAttach() #1048

Open gsmedley opened 6 years ago

gsmedley commented 6 years ago

Sometimes after debugging for a while, I get a crash when going back from a page. The error says key is not defined in NSRouteReuseStrategy.shouldAttach() here:

const shouldAttach = isBack && this.cache.peek().key === key;

I see in other places nearby there is a check that this.cache.peek() exists before calling .key.

corne-de-bruin commented 6 years ago

After upgrading our app from NativeScript 3.1 to 3.2 (or 3.3) we are facing the same issue when we use the back button on android afer the user opened a deep link.

TypeError: Cannot read property 'key' of undefined
     at NSRouteReuseStrategy.shouldAttach (file:///data/data/[appname]/files/app/tns_modules/nativescript-angular/router/ns-route-reuse-strategy.js:64:55)
     at ActivateRoutes.activateRoutes (file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4743:45)
     at file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4715:58
     at Array.forEach (native)
     at ActivateRoutes.activateChildRoutes (file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4715:29)
     at ActivateRoutes.activateRoutes (file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4736:22)
     at file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4715:58
     at Array.forEach (native)
     at ActivateRoutes.activateChildRoutes (file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4715:29)
     at ActivateRoutes.activate (file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4608:14)
     at file:///data/data/[appname]/files/app/tns_modules/@angular/router/bundles/router.umd.js:4185:22
     at SafeSubscriber._next (file:///data/data/[appname]/files/app/tns_modules/rxjs/Observable.js:224:21)
     at SafeSubscriber.__tryOrSetError (file:///data/data/[appname]/files/app/tns_modules/rxjs/Subscriber.js:247:16)
     at SafeSubscriber.next (file:///data/data/[appname]/files/app/tns_modules/rxjs/Subscriber.js:187:27)
     at Subscriber._next (file:///data/data/[appname]/files/app/tns_modules/rxjs/Subscriber.js:125:26)
dennis-montana commented 6 years ago

This is also happening in our app with NativeScript 3.2 and 3.3 on iOS. When routing to a page using a deep link and then clicking the back button:

Error: Uncaught (in promise): TypeError: undefined is not an object (evaluating 'this.cache.peek().key')

shouldAttach@file:///app/tns_modules/nativescript-angular/router/ns-route-reuse-strategy.js:64:55  
activateRoutes@file:///app/tns_modules/@angular/router/./bundles/router.umd.js:4743:57  
file:///app/tns_modules/@angular/router/./bundles/router.umd.js:4715:72  
forEach@[native code]  
activateChildRoutes@file:///app/tns_modules/@angular/router/./bundles/router.umd.js:4715:36
activateRoutes@file:///app/tns_modules/@angular/router/./bundles/router.umd.js:4736:41
file:///app/tns_modules/@angular/router/./bundles/router.umd.js:4715:72
forEach@[native code]
corne-de-bruin commented 6 years ago

After more debugging I've found that this will happen on a back route after a clear history. So after a certain action we call a route with a clear history, that route calls another one directly. If we then click the back button to go to the first route (after the clear history) we will get this error.

EDIT: After adding more logging i've found the following flow:

Router event: NavigationStart(id: 4, url: '/home')
Router event: RoutesRecognized(id: 4, url: '/home', urlAfterRedirects: '/home', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'', path:'') }  } )
Router event: GuardsCheckStart(id: 4, url: '/home', urlAfterRedirects: '/home', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'', path:'') }  } )
Router event: GuardsCheckEnd(id: 4, url: '/home', urlAfterRedirects: '/home', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'', path:'') }  } , shouldActivate: true)
Router event: ResolveStart(id: 4, url: '/home', urlAfterRedirects: '/home', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'', path:'') }  } )
Router event: ResolveEnd(id: 4, url: '/home', urlAfterRedirects: '/home', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'', path:'') }  } )
===== NSRouteReuseStrategy -> Store: push state
Router event: NavigationEnd(id: 4, url: '/home', urlAfterRedirects: '/home')
Router event: NavigationStart(id: 5, url: '/home/invoices')
Router event: RoutesRecognized(id: 5, url: '/home/invoices', urlAfterRedirects: '/home/invoices', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'invoices', path:'invoices') }  } )
Router event: GuardsCheckStart(id: 5, url: '/home/invoices', urlAfterRedirects: '/home/invoices', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'invoices', path:'invoices') }  } )
Router event: GuardsCheckEnd(id: 5, url: '/home/invoices', urlAfterRedirects: '/home/invoices', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'invoices', path:'invoices') }  } , shouldActivate: true)
Router event: ResolveStart(id: 5, url: '/home/invoices', urlAfterRedirects: '/home/invoices', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'invoices', path:'invoices') }  } )
Router event: ResolveEnd(id: 5, url: '/home/invoices', urlAfterRedirects: '/home/invoices', state: Route(url:'', path:'') { Route(url:'home', path:'home') { Route(url:'invoices', path:'invoices') }  } )
===== NSRouteReuseStrategy -> Store: push state
Router event: NavigationEnd(id: 5, url: '/home/invoices', urlAfterRedirects: '/home/invoices')
===== NSRouteReuseStrategy -> clearCache

So when we start a navigate command with the clearCache: true flag it will first push the two next routes in the cache and only after the navigation is done it will perform the clear cache. Then if you would push the back button the cache i wiped and therefore the reuse strategy can't find it anymore

EDIT 2: If I alter the loadComponentInPage function in the PageRouterOutlet to:

loadComponentInPage(page: Page, componentRef: ComponentRef<any>): void {
    // Component loaded. Find its root native view.
    const componentView = componentRef.location.nativeElement;
// Remove it from original native parent.
    this.viewUtil.removeChild(componentView.parent, componentView);
// Add it to the new page
    page.content = componentView;

    page.on(Page.navigatedFromEvent, (<any>global).Zone.current.wrap((args: NavigatedData) => {
        if (args.isBackNavigation) {
            this.locationStrategy._beginBackPageNavigation();
            this.locationStrategy.back();
        }
    }));

    const navOptions = this.locationStrategy._beginPageNavigation();

    // Clear refCache if navigation with clearHistory
    if (navOptions.clearHistory) {
        //directly call clearCache in stead of using a timeout in combination with a page event
        this.routeReuseStrategy.clearCache();
    }

    this.frame.navigate({
        create: () => {
            return page;
        },
        clearHistory: navOptions.clearHistory,
        animated: navOptions.animated,
        transition: navOptions.transition
    });
}
gogoout commented 5 years ago

This also happens to my app. When user logined, we will redirect the user to home page with a clearHistory flag. If the user then hitting whatever redirection buttons on the home page as fast as they can, they will run into this issue when they try go back.

The issue is raised by the usage of setTimeout. Any particular reason to use setTimeout?

JS: clearHistory,run setTimeout
JS: navigatingFromEvent
JS: actually clear cache
corne-de-bruin commented 5 years ago

With NS 5.3.1 and NS-Angular 7.2.3 we are seeing similar behaviour again on back routing after a clearHistory. But instead of the app crashing we end up with a blank page.

I've created a sample application that shows the same behaviour. https://play.nativescript.org/?template=play-ng&id=m84Itu

If you open this app it will navigate from the home to a second page (with a pink background) and then to a third page with a blue background. If you then navigate back the second page (pink one) is empty (blank).

In our production app I could fix this behaviour by changing the loadComponentInPage function of the PageRouterOutlet.

https://github.com/NativeScript/nativescript-angular/blob/e59f8d4ba58ade094df332d50d63227400a9427f/nativescript-angular/router/page-router-outlet.ts#L347

// Component loaded. Find its root native view.
    const componentView = componentRef.location.nativeElement;
    // Remove it from original native parent.
    this.viewUtil.removeChild(componentView.parent, componentView);
    // Add it to the new page
    page.content = componentView;

    page.on(Page.navigatedFromEvent, (<any>global).Zone.current.wrap((args: NavigatedData) => {
        if (args.isBackNavigation) {
            this.locationStrategy._beginBackPageNavigation(this.frame);
            this.locationStrategy.back(null, this.frame);
        }
    }));

    const navOptions = this.locationStrategy._beginPageNavigation(this.frame);

    // Clear refCache if navigation with clearHistory
    if (navOptions.clearHistory) {
        const clearCallback = () => () => {
            if (this.outlet) {
                this.routeReuseStrategy.clearCache(this.outlet.outletKeys[0]);
            }
            page.off(Page.navigatedToEvent, clearCallback);
        };

        page.on(Page.navigatedToEvent, clearCallback);
    }

    this.frame.navigate({
        create: () => { return page; },
        clearHistory: navOptions.clearHistory,
        animated: navOptions.animated,
        transition: navOptions.transition
    });

I just removed the setTimeout call around the callback function.

bachras commented 5 years ago

I am having same issue as well

joshcomley commented 5 years ago

I had this issue if I called frameModule.topmost().goBack() one too many times

nericode commented 4 years ago

import { RouterExtensions } from "nativescript-angular/router";

I had this issue if I called routerExtensions.back() too many times...

Uncaught (in promise): TypeError: Cannot read property 'key' of undefined
JS: TypeError: Cannot read property 'key' of undefined
JS:     at NSRouteReuseStrategy.push.../node_modules/@nativescript/angular/router/ns-route-reuse-strategy.js.NSRouteReuseStrategy.shouldAttach (file:///node_modules/@nativescript/angular/router/ns-route-reuse-strategy.js:107:0)
JS:     at ActivateRoutes.push.../node_modules/@angular/router/fesm5/router.js.ActivateRoutes.activateRoutes (file:///node_modules/@angular/router/fesm5/router.js:2288:0)
JS:     at file:///node_modules/@angular/router/fesm5/router.js:2261:0
JS:     at Array.forEach (<anonymous>)
JS:     at ActivateRoutes.push.../node_modules/@angular/router/fesm5/router.js.ActivateRoutes.activateChildRoutes (file:///node_modules/@angular/router/fesm5/router.js:2260:0)
JS:     at ActivateRoutes.push.../node_modules/@angular/router/fesm5/router.js.ActivateRoutes.activate (file:///data/data/org.nativescript.wipperappclient/files/app/ven...
JS: ERROR Error: Uncaught (in promise): TypeError: Cannot read property 'key' of undefined
JS: TypeError: Cannot read property 'key' of undefined
JS:     at NSRouteReuseStrategy.push.../node_modules/@nativescript/angular/router/ns-route-reuse-strategy.js.NSRouteReuseStrategy.shouldAttach (file:///node_modules/@nativescript/angular/router/ns-route-reuse-strategy.js:107:0)
JS:     at ActivateRoutes.push.../node_modules/@angular/router/fesm5/router.js.ActivateRoutes.activateRoutes (file:///node_modules/@angular/router/fesm5/router.js:2288:0)
JS:     at file:///node_modules/@angular/router/fesm5/router.js:2261:0
JS:     at Array.forEach (<anonymous>)
JS:     at ActivateRoutes.push.../node_modules/@angular/router/fesm5/router.js.ActivateRoutes.activateChildRoutes (file:///node_modules/@angular/router/fesm5/router.js:2260:0)
JS:     at ActivateRoutes.push.../node_modules/@angular/router/fesm5/router.js.ActivateRoutes.activate (file:///data/data/org.nativescript.wipperappclient/files/app/ven...
NazimMertBilgi commented 4 years ago

is there any improvement on this subject?

NazimMertBilgi commented 4 years ago

Workaround:

application.android.on(AndroidApplication.activityBackPressedEvent, (data: AndroidActivityBackPressedEventData) => {
  const validate = this.router.url === "/home/default/(usersListTab:users-list//messagesTab:messages//notificationsTab:notifications//myProfileTab:my-profile)";
  if (validate) {
    data.cancel = true; // prevents default back button behavior
  }
});