fix(module-navigation): check previous state before dispatching push …

…and replace (#1467)
equinor · Oct 30, 2023 · a8f0f06 · a8f0f06
1 parent 90addab
commit a8f0f06
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 29 deletions.
diff --git a/.changeset/cool-countries-doubt.md b/.changeset/cool-countries-doubt.md
@@ -0,0 +1,5 @@
+---
+'@equinor/fusion-framework-module-navigation': patch
+---
+
+checks when `push`|`replace` is called if to is identical to previous to
diff --git a/packages/modules/navigation/src/configurator.ts b/packages/modules/navigation/src/configurator.ts
@@ -8,10 +8,4 @@ export interface INavigationConfigurator {
 export class NavigationConfigurator implements INavigationConfigurator {
     public history?: History;
     public basename?: string;
-    /**
-     * indicates if the navigator should behave as a slave or master
-     *
-     * When in `slave` mode the navigator will change push to replace (since master handles pushing history)
-     */
-    mode?: 'MASTER' | 'SLAVE';
 }
diff --git a/packages/modules/navigation/src/module.ts b/packages/modules/navigation/src/module.ts
@@ -27,9 +27,6 @@ export const module: NavigationModule = {
         const configurator = new NavigationConfigurator();
         if (ref) {
             configurator.history = (ref as ModuleInstance).navigation.navigator;
-            configurator.mode = 'SLAVE';
-        } else {
-            configurator.mode = 'MASTER';
         }
         return configurator;
     },

diff --git a/packages/modules/navigation/src/navigator.ts b/packages/modules/navigation/src/navigator.ts
@@ -1,4 +1,4 @@
-import type { History, To, Path } from '@remix-run/router';
+import type { History, To, Path, Location } from '@remix-run/router';
 import { BehaviorSubject, Observable, Subscription } from 'rxjs';
 
 import type { NavigationUpdate, NavigationListener } from './types';
@@ -13,6 +13,8 @@ export interface INavigator<T extends NavigationUpdate = NavigationUpdate>
     dispose: VoidFunction;
 }
 
+const isLocation = (to: To): to is Location => typeof to === 'object' && 'key' in to;
+
 export class Navigator<T extends NavigationUpdate = NavigationUpdate>
     extends Observable<T>
     implements INavigator<T>
@@ -56,7 +58,7 @@ export class Navigator<T extends NavigationUpdate = NavigationUpdate>
         mode?: 'MASTER' | 'SLAVE';
     }) {
         super((subscriber) => this.#state.subscribe(subscriber));
-        const { basename, history, mode } = args;
+        const { basename, history } = args;
         this.#logger = args.logger || console;
         this.#baseName = basename;
         this.#history = history;
@@ -67,21 +69,8 @@ export class Navigator<T extends NavigationUpdate = NavigationUpdate>
         } as T);
         this.#subscriptions.add(
             this.#history.listen((update) => {
-                /** prevent duplicate navigation */
-                if (update.location.key === this.#state.value.location.key) {
-                    return;
-                }
-                this.#logger.debug('Navigator::#history.listen', {
-                    update,
-                    current: this.#state.value,
-                });
-                if (update.action === 'PUSH' && mode !== 'MASTER') {
-                    this.#logger.debug(
-                        'Navigator::#history.listen',
-                        'switching action to [REPLACE], since navigator is not master',
-                    );
-                    this.#state.next({ ...update, action: 'REPLACE' } as T);
-                } else {
+                /** prevent duplicate state */
+                if (update.location.key !== this.#state.value.location.key) {
                     this.#state.next(update as T);
                 }
             }),
@@ -94,8 +83,9 @@ export class Navigator<T extends NavigationUpdate = NavigationUpdate>
 
     public listen(listener: NavigationListener): VoidFunction {
         const subscription = this.#state.subscribe(listener);
+        this.#logger.debug('Navigator::listen', listener);
         return () => {
-            this.#logger.debug('Navigator::listen[unsubscribe]');
+            this.#logger.debug('Navigator::listen[unsubscribe]', listener);
             return subscription.unsubscribe();
         };
     }
@@ -109,17 +99,28 @@ export class Navigator<T extends NavigationUpdate = NavigationUpdate>
     }
 
     public push(to: To, state?: unknown): void {
-        return this.#history.push(to, state);
+        const skip = isLocation(to) && this._isDuplicateLocation(to);
+        if (!skip) {
+            return this.#history.push(to, state);
+        }
     }
 
     public replace(to: To, state?: unknown): void {
-        return this.#history.replace(to, state);
+        const skip = isLocation(to) && this._isDuplicateLocation(to);
+        if (!skip) {
+            return this.#history.replace(to, state);
+        }
     }
 
     public createURL(to: To): URL {
         return this.#history.createURL(to);
     }
 
+    protected _isDuplicateLocation(location: Location) {
+        // TODO: might check if the are changes in url compared to current
+        return location.key === this.#state.value.location.key;
+    }
+
     dispose() {
         this.#subscriptions.unsubscribe();
     }