Merge pull request #1020 from Quetzacoalt91/safe-script-dismount

Add test to show issues from a script unloaded too late
PrestaShop · Nov 20, 2024 · b31fa33 · b31fa33
2 parents 22a6bb9 + 6704cb8
commit b31fa33
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 12 deletions.
diff --git a/_dev/src/ts/routing/ScriptHandler.ts b/_dev/src/ts/routing/ScriptHandler.ts
@@ -68,4 +68,15 @@ export default class ScriptHandler {
     this.#currentScript?.beforeDestroy();
     this.#loadScript(newRoute);
   }
+
+  /**
+   * @public
+   * @returns void
+   * @description Unloads the currently loaded script.
+   *  Should be called before updating the DOM.
+   */
+  public unloadRouteScript(): void {
+    this.#currentScript?.beforeDestroy();
+    this.#currentScript = undefined;
+  }
 }
diff --git a/_dev/src/ts/utils/Hydration.ts b/_dev/src/ts/utils/Hydration.ts
@@ -27,6 +27,10 @@ export default class Hydration {
     const elementToUpdate = document.getElementById(data.parent_to_update);
 
     if (elementToUpdate && data.new_content) {
+      if (data.new_route) {
+        scriptHandler.unloadRouteScript();
+      }
+
       elementToUpdate.innerHTML = data.new_content;
 
       if (data.new_route) {

diff --git a/_dev/tests/utils/Hydration.test.ts b/_dev/tests/utils/Hydration.test.ts
@@ -1,15 +1,34 @@
 import Hydration from '../../src/ts/utils/Hydration';
 import { ApiResponseHydration } from '../../src/ts/types/apiTypes';
-import { routeHandler, scriptHandler } from '../../src/ts/autoUpgrade';
+import RouteHandler from '../../src/ts/routing/RouteHandler';
+import ScriptHandler from '../../src/ts/routing/ScriptHandler';
+
+const setNewRouteMock = jest.spyOn(RouteHandler.prototype, 'setNewRoute');
+const updateRouteScriptMock = jest.spyOn(ScriptHandler.prototype, 'updateRouteScript');
+const unloadRouteScriptMock = jest.spyOn(ScriptHandler.prototype, 'unloadRouteScript');
+
+jest.mock('../../src/ts/pages/HomePage', () => {
+  return jest.fn().mockImplementation(() => {
+    return {
+      mount: () => {},
+      beforeDestroy: () => {}
+    };
+  });
+});
 
-jest.mock('../../src/ts/autoUpgrade', () => ({
-  routeHandler: {
-    setNewRoute: jest.fn()
-  },
-  scriptHandler: {
-    updateRouteScript: jest.fn()
-  }
-}));
+jest.mock('../../src/ts/pages/UpdatePageBackup', () => {
+  return jest.fn().mockImplementation(() => ({
+    mount: () => {},
+    beforeDestroy: () => {
+      const element = document.getElementById('my_paragraph');
+      if (!element) {
+        throw new Error(
+          'Script unloaded too late, the element has already been removed from the DOM'
+        );
+      }
+    }
+  }));
+});
 
 describe('Hydration', () => {
   let hydration: Hydration;
@@ -51,7 +70,7 @@ describe('Hydration', () => {
 
     hydration.hydrate(response);
 
-    expect(scriptHandler.updateRouteScript).toHaveBeenCalledWith('new_route_value');
+    expect(updateRouteScriptMock).toHaveBeenCalledWith('new_route_value');
   });
 
   it('should call routeHandler.setNewRoute when new_route is provided and fromPopState is false', () => {
@@ -64,7 +83,7 @@ describe('Hydration', () => {
 
     hydration.hydrate(response);
 
-    expect(routeHandler.setNewRoute).toHaveBeenCalledWith('new_route_value');
+    expect(setNewRouteMock).toHaveBeenCalledWith('new_route_value');
   });
 
   it('should not call routeHandler.setNewRoute when fromPopState is true', () => {
@@ -77,7 +96,7 @@ describe('Hydration', () => {
 
     hydration.hydrate(response, true);
 
-    expect(routeHandler.setNewRoute).not.toHaveBeenCalled();
+    expect(setNewRouteMock).not.toHaveBeenCalled();
   });
 
   it('should not update the content if the element does not exist', () => {
@@ -115,3 +134,44 @@ describe('Hydration', () => {
     );
   });
 });
+
+describe('Hydration and scripts lifecycle', () => {
+  let hydration: Hydration;
+
+  beforeEach(() => {
+    hydration = new Hydration();
+    document.body.innerHTML = `
+      <div id="parent">
+        <p>Old Content</p>
+      </div>
+    `;
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should unload the current script safely before loading the next one', () => {
+    const initialResponse: ApiResponseHydration = {
+      hydration: true,
+      new_content: `<p id="my_paragraph">Old Content</p>`,
+      parent_to_update: 'parent',
+      new_route: 'update-page-backup'
+    };
+    hydration.hydrate(initialResponse);
+
+    expect(setNewRouteMock).toHaveBeenCalledTimes(1);
+    expect(unloadRouteScriptMock).toHaveBeenCalledTimes(1);
+
+    const nextResponse: ApiResponseHydration = {
+      hydration: true,
+      new_content: `<p>New Content</p>`,
+      parent_to_update: 'parent',
+      new_route: 'home-page'
+    };
+    hydration.hydrate(nextResponse);
+
+    expect(setNewRouteMock).toHaveBeenCalledTimes(2);
+    expect(unloadRouteScriptMock).toHaveBeenCalledTimes(2);
+  });
+});