Merge pull request adobe#81 from adobe/issue/history-routing

fix: window.history updated only on init
marto · May 31, 2022 · bd5e432 · bd5e432
2 parents d1ceca6 + 323930c
commit bd5e432
Show file tree

Hide file tree

Showing 9 changed files with 22,900 additions and 12,568 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -16,18 +16,18 @@
     "url": "https://github.com/adobe/aem-spa-page-model-manager/issues"
   },
   "engines": {
-    "npm": "6.14.15",
+    "npm": ">=6.14.15",
     "node": ">=12.16.2"
   },
   "main": "dist/aem-spa-page-model-manager.js",
   "types": "dist/types.d.ts",
   "scripts": {
     "build:production": "NODE_ENV=production npm run build",
-    "build:types": "tsc -p src/tsconfig.types.json",
-    "build": "npm run clean && npm run linter && webpack && npm run build:types",
+    "build:types": "tsc -p tsconfig.types.json",
+    "build": "npm run clean && npm run lint && npm run build:types && webpack",
     "clean": "rm -rf dist/",
     "docs": "npm i && npx typedoc --excludePrivate ./src --out ./dist/docs",
-    "linter": "eslint .",
+    "lint": "eslint .",
     "semantic-release": "semantic-release",
     "test:coverage": "jest --clearCache && jest --coverage",
     "test:debug": "jest --coverage --watchAll",
@@ -40,23 +40,23 @@
   },
   "devDependencies": {
     "@adobe/eslint-config-editorxp": "^1.0.3",
-    "@semantic-release/changelog": "^5.0.1",
-    "@semantic-release/git": "^9.0.0",
-    "@semantic-release/github": "^7.1.1",
+    "@semantic-release/changelog": "^6.0.1",
+    "@semantic-release/git": "^10.0.1",
+    "@semantic-release/github": "^8.0.4",
     "@types/clone": "^2.1.0",
     "@types/jest": "^26.0.14",
     "@types/node": "^14.11.5",
     "@typescript-eslint/eslint-plugin": "^4.4.0",
     "@typescript-eslint/parser": "^4.4.0",
     "clean-webpack-plugin": "^3.0.0",
-    "commitizen": "^4.2.1",
-    "cz-conventional-changelog": "^3.3.0",
+    "commitizen": "^4.2.4",
+    "cz-conventional-changelog": "^3.0.1",
     "eslint": "^7.10.0",
     "eslint-plugin-header": "^3.1.0",
     "eslint-plugin-json": "^2.1.2",
     "jest": "^26.5.2",
     "jest-fetch-mock": "^3.0.3",
-    "semantic-release": "^17.1.2",
+    "semantic-release": "^19.0.2",
     "ts-jest": "^26.4.1",
     "ts-loader": "^8.1.0",
     "ts-mockito": "^2.6.1",

diff --git a/src/ModelManager.ts b/src/ModelManager.ts
@@ -18,7 +18,7 @@ import { ModelClient } from './ModelClient';
 import { ModelStore } from './ModelStore';
 import { PathUtils } from './PathUtils';
 import { AuthoringUtils } from './AuthoringUtils';
-import { isRouteExcluded } from './ModelRouter';
+import { initModelRouter, isRouteExcluded } from './ModelRouter';
 
 /**
  * Checks whether provided child path exists in the model.
@@ -205,6 +205,8 @@ export class ModelManager {
         if (rootModelPath) {
             this._setInitializationPromise(rootModelPath);
         }
+
+        initModelRouter();
     }
 
     /**

diff --git a/src/ModelRouter.ts b/src/ModelRouter.ts
@@ -16,7 +16,8 @@ import ModelManager from './ModelManager';
 import { PathUtils } from './PathUtils';
 
 /**
- * <p>The ModelRouter listens for HTML5 History API <i>popstate</i> events and calls {@link PageModelManager#getData()} with the model path it extracted from the URL.</p>
+ * <p>The ModelRouter listens for HTML5 History API <i>popstate</i> events
+ * and calls {@link PageModelManager#getData()} with the model path it extracted from the URL.</p>
  *
  * <h2>Configuration</h2>
  *
@@ -74,8 +75,8 @@ export class RouterModes {
  * @private
  * @return
  */
-export function getModelPath(url?: string | null): string | null {
-    const localUrl = url || window.location.pathname;
+export function getModelPath(url?: string | null | URL): string | null {
+    const localUrl = (url || window.location.pathname) as string;
 
     return PathUtils.sanitize(localUrl);
 }
@@ -159,7 +160,7 @@ export function dispatchRouteChanged(path: string): void {
  *
  * @private
  */
-export function routeModel(url?: string | null): void {
+export function routeModel(url?: string | undefined | null | URL): void {
     if (!isModelRouterEnabled()) {
         return;
     }
@@ -176,28 +177,29 @@ export function routeModel(url?: string | null): void {
     dispatchRouteChanged(path);
 }
 
-// Activate the model router
-if (isModelRouterEnabled()) {
-    // Encapsulate the history.pushState and history.replaceState functions to prefetch the page model for the current route
-    const pushState = window.history.pushState;
-    const replaceState = window.history.replaceState;
+export function initModelRouter(): void {
+    // Activate the model router
+    if (isModelRouterEnabled() && typeof window !== 'undefined') {
+        // Encapsulate the history.pushState and history.replaceState functions to prefetch the page model for the current route
+        const pushState = window.history.pushState;
+        const replaceState = window.history.replaceState;
 
-    window.history.pushState = (state, title, url) => {
-        routeModel(url || null);
+        window.addEventListener('popstate', e => {
+            const target = e?.target as Window;
 
-        return pushState.apply(history, [ state, title, url ]);
-    };
-
-    window.history.replaceState = (state, title, url) => {
-        routeModel(url || null);
+            routeModel(target?.location?.pathname || null);
+        });
 
-        return replaceState.apply(history, [ state, title, url ]);
-    };
+        window.history.pushState = (state, title, url) => {
+            routeModel(url);
 
-    window.onpopstate = (history: any) => {
-        const currentPath = history?.target?.location?.pathname;
+            return pushState.apply(history, [ state, title, url ]);
+        };
 
-        routeModel(currentPath || null);
-    };
+        window.history.replaceState = (state, title, url) => {
+            routeModel(url || null);
 
-}
+            return replaceState.apply(history, [ state, title, url ]);
+        };
+    }
+}
diff --git a/src/PathUtils.ts b/src/PathUtils.ts
@@ -191,7 +191,7 @@ export class PathUtils {
      * to make sure only properly formatted paths (e.g., "/content/mypage") are stored.
      * @param path - Path of the page to be sanitized.
      */
-    public static sanitize(path: string | null) {
+    public static sanitize(path: string | null): string | null {
         if (!path || (typeof path !== 'string')) {
             return null;
         }
@@ -231,17 +231,14 @@ export class PathUtils {
         if (!extension || extension.length < 1) {
             return path;
         }
-
         if (!extension.startsWith('.')) {
             extension = `.${extension}`;
         }
-
         if (!path || (path.length < 1) || (path.indexOf(extension) > -1)) {
             return path;
         }
 
         let extensionPath = this.normalize(path);
-
         const url = new URL(extensionPath, DUMMY_ORIGIN);
         let resourcePath = this.sanitize(url.pathname);
 
@@ -301,7 +298,7 @@ export class PathUtils {
      * @param path - Path to be extended.
      * @param selector - Selector to be added.
      */
-    public static addSelector(path: string, selector: string) {
+    public static addSelector(path: string, selector: string): string {
         if (!selector || (selector.length < 1)) {
             return path;
         }
@@ -390,7 +387,7 @@ export class PathUtils {
     /**
      * Returns path to the direct parent.
      */
-    public static getParentNodePath(path: string | null) {
+    public static getParentNodePath(path: string | null): string | null {
         if (path && (path.length > 0)) {
             const splashIndex = path.lastIndexOf('/') + 1;
 
@@ -423,7 +420,7 @@ export class PathUtils {
      * Returns the subpath of the targetPath relative to the rootPath,
      * or the targetPath if the rootPath is not a root of the targetPath.
      */
-    public static subpath(targetPath?: string, rootPath?: string) {
+    public static subpath(targetPath?: string, rootPath?: string): string {
         if (!targetPath) {
             return '';
         }
@@ -445,9 +442,9 @@ export class PathUtils {
 
         if (index === rootPathChildren.length) {
             return targetPathChildren.slice(index).join('/');
-        } else {
-            return targetPath;
         }
+
+        return targetPath;
     }
 
     /**
@@ -500,12 +497,14 @@ export class PathUtils {
 
         const splitPaths = path.split(`/${Constants.JCR_CONTENT}/`);
 
-        const split = {
+        const split:{
+            pagePath: string;
+            itemPath?: string;
+        } = {
             pagePath: splitPaths[0]
         };
 
         if (splitPaths.length > 1) {
-            // @ts-ignore
             split.itemPath = splitPaths[1];
         }
 

diff --git a/test/ModelRouter.test.ts b/test/ModelRouter.test.ts
@@ -13,7 +13,9 @@
 import MetaProperty from '../src/MetaProperty';
 import { Model } from '../src/Model';
 import ModelManager from '../src/ModelManager';
-import { dispatchRouteChanged, getModelPath, getRouteFilters, isModelRouterEnabled, isRouteExcluded, routeModel, RouterModes } from '../src/ModelRouter';
+import {
+    dispatchRouteChanged, getModelPath, getRouteFilters, initModelRouter, isModelRouterEnabled, isRouteExcluded, routeModel, RouterModes,
+} from '../src/ModelRouter';
 import { PathUtils } from '../src/PathUtils';
 
 let metaProps: { [key: string]: string } = {};
@@ -146,4 +148,35 @@ describe('ModelRouter ->', () => {
             expect(isRouteExcluded(MODEL_ROUTE_FILTERS[2])).toEqual(true);
         });
     });
+
+    describe('router model on history ->', () => {
+        const originalLocation = window.location;
+
+        beforeEach(() => {
+            initModelRouter();
+            modelManagerSpy.mockResolvedValue(TEST_MODEL as Model);
+        });
+
+        afterAll(() => {
+            Object.defineProperty(window, 'location', { configurable: true, value: originalLocation });
+            modelManagerSpy.mockReset();
+        });
+        it('should fetch model on history push', () => {
+            window.history.pushState({}, '', '/test');
+            expect(modelManagerSpy).toHaveBeenCalledWith({ path: '/test' });
+        });
+        it('should fetch model on history replace', () => {
+            window.history.replaceState({}, '', '/test');
+            expect(modelManagerSpy).toHaveBeenCalledWith({ path: '/test' });
+        });
+        it('should fetch model on history pop', () => {
+            Object.defineProperty(window, 'location', {
+                configurable: true,
+                value: { pathname: '/test' }
+            });
+            window.dispatchEvent(new Event('popstate'));
+            expect(modelManagerSpy).toHaveBeenCalledWith({ path: '/test' });
+        });
+
+    });
 });
diff --git a/src/tsconfig.base.json → tsconfig.base.json b/src/tsconfig.base.json → tsconfig.base.json
@@ -1,5 +1,5 @@
 {
-    "extends": "../tsconfig.json",
+    "extends": "./tsconfig.json",
     "compilerOptions": {
         "noEmit": false,
         "baseUrl": ".",

diff --git a/tsconfig.json b/tsconfig.json
@@ -22,5 +22,5 @@
   },
   "include": [
     "src/**/*"
-  ]
+, "tsconfig.base.json"  ]
 }
diff --git a/src/tsconfig.types.json → tsconfig.types.json b/src/tsconfig.types.json → tsconfig.types.json
@@ -8,7 +8,7 @@
         "removeComments": false,
         "declaration": true,
         "declarationMap": true,
-        "declarationDir": "../dist",
+        "declarationDir": "./dist",
         "emitDeclarationOnly": true
     },
     "exclude": [