fix: read navigation extras from the router navigation instead of using stored values (#88)

* fix: read navigation extras from the router navigation instead of using stored values

* feat: remove navigation extras logic from location strategy

* chore: fix linting issues

* fix: move inject to constructor

Author: edusperoni

apps/nativescript-demo-ng/src/test.ts

@@ -1,7 +1,13 @@
+// eslint-disable-next-line @typescript-eslint/triple-slash-reference
+/// <reference path="../../../node_modules/webpack/module.d.ts" />
 import { runTestApp } from '@nativescript/unit-test-runner';
 // import other polyfills here
 
-declare let require: any;
+// polyfills required for latest karma
+global.addEventListener ??= () => {
+  // do nothing as there`s no global event listener
+};
+global.queueMicrotask ??= (fn: () => unknown) => Promise.resolve().then(fn);
 
 runTestApp({
   runTests: () => {

apps/nativescript-demo-ng/src/tests/ns-location-strategy.spec.ts

@@ -1,6 +1,6 @@
 // make sure you import mocha-config before @angular/core
 import { DefaultUrlSerializer, UrlTree } from '@angular/router';
-import { FrameService, LocationState, NSLocationStrategy, Outlet } from '@nativescript/angular';
+import { FrameService, LocationState, NavigationOptions, NSLocationStrategy, Outlet } from '@nativescript/angular';
 import { BackstackEntry, Frame, NavigationEntry, Page, View } from '@nativescript/core';
 
 export class FakeFrameService extends FrameService {
@@ -134,13 +134,13 @@ function createState(url: string, outletName: string, isPageNav: boolean = false
   };
 }
 
-function simulatePageNavigation(strategy: NSLocationStrategy, url: string, frame: any, outletName?: string) {
+function simulatePageNavigation(strategy: NSLocationStrategy, url: string, frame: any, outletName?: string, options?: NavigationOptions) {
   outletName = outletName || 'primary';
   strategy.pushState(null, null, url, null);
 
   const outlet: Outlet = strategy.findOutlet(outletName);
   outlet.frames.push(frame);
-  strategy._beginPageNavigation(frame);
+  strategy._beginPageNavigation(frame, options);
 }
 
 function simulatePageBack(strategy: NSLocationStrategy, frame: any) {
@@ -479,8 +479,7 @@ describe('NSLocationStrategy', () => {
     const frame = new FakeFrame();
     const outletName = 'primary';
     // Act
-    strategy._setNavigationOptions({ clearHistory: true });
-    simulatePageNavigation(strategy, '/cleared', frame, outletName);
+    simulatePageNavigation(strategy, '/cleared', frame, outletName, { clearHistory: true });
     const outlet: Outlet = strategy.findOutlet(outletName);
     // Assert
     assertStatesEqual(outlet.states, [createState('/cleared', outletName, true)]);
@@ -494,8 +493,7 @@ describe('NSLocationStrategy', () => {
     const outletName2 = 'test2';
 
     // Act
-    strategy._setNavigationOptions({ clearHistory: true });
-    simulatePageNavigation(strategy, '/(test1:cleared//test2:test2)', frame, outletName);
+    simulatePageNavigation(strategy, '/(test1:cleared//test2:test2)', frame, outletName, { clearHistory: true });
     simulatePageNavigation(strategy, '/(test1:cleared//test2:test2)', frame2, outletName2);
     const expectedStatesTest1: Array<LocationState> = [createState('/(test1:cleared//test2:test2)', outletName, true)];
     const expectedStatesTest2: Array<LocationState> = [createState('/(test1:cleared//test2:test2)', outletName2, true)];

packages/angular/src/lib/legacy/router/ns-location-strategy.ts

@@ -1,12 +1,11 @@
-import { Inject, Injectable, Optional } from '@angular/core';
 import { LocationChangeEvent, LocationStrategy } from '@angular/common';
-import { DefaultUrlSerializer, UrlSegmentGroup, UrlTree, ActivatedRouteSnapshot, Params } from '@angular/router';
+import { Inject, Injectable, Optional } from '@angular/core';
+import { ActivatedRouteSnapshot, DefaultUrlSerializer, Params, UrlSegmentGroup, UrlTree } from '@angular/router';
 import { Frame } from '@nativescript/core';
+import { START_PATH } from '../../tokens';
 import { NativeScriptDebug } from '../../trace';
-import { isPresent } from '../../utils/lang-facade';
 import { FrameService } from '../frame.service';
-import { Outlet, NavigationOptions, LocationState, defaultNavOptions } from './ns-location-utils';
-import { START_PATH } from '../../tokens';
+import { defaultNavOptions, LocationState, NavigationOptions, Outlet } from './ns-location-utils';
 
 @Injectable({
   providedIn: 'root',
@@ -16,7 +15,6 @@ export class NSLocationStrategy extends LocationStrategy {
   private currentOutlet: Outlet;
 
   private popStateCallbacks = new Array<(_: any) => any>();
-  private _currentNavigationOptions: NavigationOptions;
   private currentUrlTree: UrlTree;
 
   public _modalNavigationDepth = 0;
@@ -42,8 +40,8 @@ export class NSLocationStrategy extends LocationStrategy {
       return '/';
     }
 
-    let tree = this.currentUrlTree;
-    let changedOutlet = this.getSegmentGroupByOutlet(this.currentOutlet);
+    const tree = this.currentUrlTree;
+    const changedOutlet = this.getSegmentGroupByOutlet(this.currentOutlet);
 
     // Handle case where the user declares a component at path "/".
     // The url serializer doesn't parse this url as having a primary outlet.
@@ -110,7 +108,7 @@ export class NSLocationStrategy extends LocationStrategy {
         currentSegmentGroup.root = urlTreeRoot;
 
         const outletPath = this.getSegmentGroupFullPath(currentTree);
-        let outletKey = this.getOutletKey(outletPath, outletName);
+        const outletKey = this.getOutletKey(outletPath, outletName);
         let outlet = this.findOutlet(outletKey);
 
         const parentOutletName = currentTree.outlet || '';
@@ -188,7 +186,7 @@ export class NSLocationStrategy extends LocationStrategy {
       }
       this.callPopState(state, true);
     } else {
-      let state = this.currentOutlet.peekState();
+      const state = this.currentOutlet.peekState();
       if (state && state.isPageNavigation) {
         // This was a page navigation - so navigate through frame.
         if (NativeScriptDebug.isLogEnabled()) {
@@ -237,7 +235,7 @@ export class NSLocationStrategy extends LocationStrategy {
   private callPopState(state: LocationState, pop: boolean = true, outlet?: Outlet) {
     outlet = outlet || this.currentOutlet;
     const urlSerializer = new DefaultUrlSerializer();
-    let changedOutlet = this.getSegmentGroupByOutlet(outlet);
+    const changedOutlet = this.getSegmentGroupByOutlet(outlet);
 
     if (state && changedOutlet) {
       this.updateSegmentGroup(this.currentUrlTree.root, changedOutlet, state.segmentGroup);
@@ -249,7 +247,7 @@ export class NSLocationStrategy extends LocationStrategy {
 
     const url = urlSerializer.serialize(this.currentUrlTree);
     const change: LocationChangeEvent = { state, type: 'popstate' };
-    for (let fn of this.popStateCallbacks) {
+    for (const fn of this.popStateCallbacks) {
       fn(change);
     }
   }
@@ -359,7 +357,7 @@ export class NSLocationStrategy extends LocationStrategy {
     }
   }
 
-  public _beginPageNavigation(frame: Frame): NavigationOptions {
+  public _beginPageNavigation(frame: Frame, options?: NavigationOptions): NavigationOptions {
     if (NativeScriptDebug.isLogEnabled()) {
       NativeScriptDebug.routerLog('NSLocationStrategy._beginPageNavigation()');
     }
@@ -371,31 +369,17 @@ export class NSLocationStrategy extends LocationStrategy {
       lastState.isPageNavigation = true;
     }
 
-    const navOptions = this._currentNavigationOptions || defaultNavOptions;
+    const navOptions = options || defaultNavOptions;
+
     if (navOptions.clearHistory) {
       if (NativeScriptDebug.isLogEnabled()) {
         NativeScriptDebug.routerLog('NSLocationStrategy._beginPageNavigation clearing states history');
       }
       this.currentOutlet.states = [lastState];
     }
-
-    this._currentNavigationOptions = undefined;
     return navOptions;
   }
 
-  public _setNavigationOptions(options: NavigationOptions) {
-    this._currentNavigationOptions = {
-      clearHistory: isPresent(options.clearHistory) ? options.clearHistory : false,
-      animated: isPresent(options.animated) ? options.animated : true,
-      transition: options.transition,
-      replaceUrl: options.replaceUrl,
-    };
-
-    if (NativeScriptDebug.isLogEnabled()) {
-      NativeScriptDebug.routerLog('NSLocationStrategy._setNavigationOptions(' + `${JSON.stringify(this._currentNavigationOptions)})`);
-    }
-  }
-
   public _getOutlets(): Array<Outlet> {
     return this.outlets;
   }
@@ -501,7 +485,7 @@ export class NSLocationStrategy extends LocationStrategy {
 
   findOutlet(outletKey: string, activatedRouteSnapshot?: ActivatedRouteSnapshot): Outlet {
     let outlet: Outlet = this.outlets.find((currentOutlet) => {
-      let equalModalDepth = currentOutlet.modalNavigationDepth === this._modalNavigationDepth;
+      const equalModalDepth = currentOutlet.modalNavigationDepth === this._modalNavigationDepth;
       return equalModalDepth && currentOutlet.outletKeys.indexOf(outletKey) > -1;
     });
 
@@ -510,7 +494,7 @@ export class NSLocationStrategy extends LocationStrategy {
     if (!outlet && activatedRouteSnapshot) {
       const pathByOutlets = this.getPathByOutlets(activatedRouteSnapshot);
       outlet = this.outlets.find((currentOutlet) => {
-        let equalModalDepth = currentOutlet.modalNavigationDepth === this._modalNavigationDepth;
+        const equalModalDepth = currentOutlet.modalNavigationDepth === this._modalNavigationDepth;
         return equalModalDepth && currentOutlet.pathByOutlets === pathByOutlets;
       });
     }
@@ -520,7 +504,7 @@ export class NSLocationStrategy extends LocationStrategy {
 
   private findOutletByModal(modalNavigation: number, isShowingModal?: boolean): Outlet {
     return this.outlets.find((outlet) => {
-      let equalModalDepth = outlet.modalNavigationDepth === modalNavigation;
+      const equalModalDepth = outlet.modalNavigationDepth === modalNavigation;
       return isShowingModal ? equalModalDepth && outlet.showingModal : equalModalDepth;
     });
   }

packages/angular/src/lib/legacy/router/page-router-outlet.ts

@@ -1,5 +1,5 @@
-import { Attribute, ChangeDetectorRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, Directive, Inject, InjectionToken, Injector, OnDestroy, EventEmitter, Output, Type, ViewContainerRef, ElementRef, InjectFlags, NgZone, EnvironmentInjector } from '@angular/core';
-import { ActivatedRoute, ActivatedRouteSnapshot, ChildrenOutletContexts, Data, PRIMARY_OUTLET, RouterOutletContract } from '@angular/router';
+import { Attribute, ChangeDetectorRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, Directive, Inject, InjectionToken, Injector, OnDestroy, EventEmitter, Output, Type, ViewContainerRef, ElementRef, InjectFlags, NgZone, EnvironmentInjector, inject } from '@angular/core';
+import { ActivatedRoute, ActivatedRouteSnapshot, ChildrenOutletContexts, Data, PRIMARY_OUTLET, Router, RouterOutletContract } from '@angular/router';
 
 import { Frame, Page, NavigatedData, profile, NavigationEntry } from '@nativescript/core';
 
@@ -10,11 +10,12 @@ import { NativeScriptDebug } from '../../trace';
 import { DetachedLoader } from '../../cdk/detached-loader';
 import { ViewUtil } from '../../view-util';
 import { NSLocationStrategy } from './ns-location-strategy';
-import { Outlet } from './ns-location-utils';
+import { defaultNavOptions, Outlet } from './ns-location-utils';
 import { NSRouteReuseStrategy } from './ns-route-reuse-strategy';
 import { findTopActivatedRouteNodeForOutlet, pageRouterActivatedSymbol, loaderRefSymbol, destroyComponentRef } from './page-router-outlet-utils';
 import { registerElement } from '../../element-registry';
 import { PageService } from '../../cdk/frame-page/page.service';
+import { ExtendedNavigationExtras } from './router-extensions';
 
 export class PageRoute {
   activatedRoute: BehaviorSubject<ActivatedRoute>;
@@ -128,6 +129,7 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract {
     @Inject(PAGE_FACTORY) private pageFactory: PageFactory,
     private routeReuseStrategy: NSRouteReuseStrategy,
     private ngZone: NgZone,
+    private router: Router,
     elRef: ElementRef,
     viewUtil: ViewUtil
   ) {
@@ -404,7 +406,8 @@ export class PageRouterOutlet implements OnDestroy, RouterOutletContract {
       }
     });
 
-    const navOptions = this.locationStrategy._beginPageNavigation(this.frame);
+    const navOptions = { ...defaultNavOptions, ...(this.router.getCurrentNavigation().extras || {}) } as ExtendedNavigationExtras;
+    this.locationStrategy._beginPageNavigation(this.frame, navOptions);
     const isReplace = navOptions.replaceUrl && !navOptions.clearHistory;
 
     // Clear refCache if navigation with clearHistory

packages/angular/src/lib/legacy/router/router-extensions.ts

@@ -1,12 +1,13 @@
 import { Injectable } from '@angular/core';
-import { Router, UrlTree, NavigationExtras, ActivatedRoute } from '@angular/router';
+import { ActivatedRoute, NavigationBehaviorOptions, NavigationExtras, Router, UrlTree } from '@angular/router';
+import { NativeScriptDebug } from '../../trace';
+import { FrameService } from '../frame.service';
 import { NSLocationStrategy } from './ns-location-strategy';
 import { NavigationOptions, Outlet } from './ns-location-utils';
-import { FrameService } from '../frame.service';
-import { NativeScriptDebug } from '../../trace';
 import { findTopActivatedRouteNodeForOutlet } from './page-router-outlet-utils';
 
 export type ExtendedNavigationExtras = NavigationExtras & NavigationOptions;
+export type ExtendedNavigationBehaviorOptions = NavigationBehaviorOptions & NavigationOptions;
 
 export interface BackNavigationOptions {
   outlets?: Array<string>;
@@ -20,17 +21,11 @@ export class RouterExtensions {
   constructor(public router: Router, public locationStrategy: NSLocationStrategy, public frameService: FrameService) {}
 
   public navigate(commands: any[], extras?: ExtendedNavigationExtras): Promise<boolean> {
-    if (extras) {
-      this.locationStrategy._setNavigationOptions(extras);
-    }
     return this.router.navigate(commands, extras);
   }
 
-  public navigateByUrl(url: string | UrlTree, options?: NavigationOptions): Promise<boolean> {
-    if (options) {
-      this.locationStrategy._setNavigationOptions(options);
-    }
-    return this.router.navigateByUrl(url);
+  public navigateByUrl(url: string | UrlTree, options?: ExtendedNavigationBehaviorOptions): Promise<boolean> {
+    return this.router.navigateByUrl(url, options);
   }
 
   public back(backNavigationOptions?: BackNavigationOptions) {