Merge pull request #619 from namecheap/fix/embedded_app_remove_css

fix(ilc): fix removing css link for embedded app
namecheap · Oct 15, 2024 · 618d971 · 618d971
2 parents b7cb10f + bae0783
commit 618d971
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 8 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -231,7 +231,7 @@ jobs:
 
   e2e_tests:
     name: Run E2E tests
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04
     strategy:
       fail-fast: false
       matrix:

diff --git a/ilc/client/CssTrackedApp.js b/ilc/client/CssTrackedApp.js
@@ -1,7 +1,11 @@
+import ilcEvents from './constants/ilcEvents';
+
 export class CssTrackedApp {
     #originalApp;
     #cssLinkUri;
     #delayCssRemoval;
+    #isRouteChanged = false;
+    #routeChangeListener;
 
     static linkUsagesAttribute = 'data-ilc-usages';
     static markedForRemovalAttribute = 'data-ilc-remove';
@@ -17,6 +21,11 @@ export class CssTrackedApp {
         // real life might differ at some time
         this.#cssLinkUri = cssLink;
         this.#delayCssRemoval = delayCssRemoval;
+
+        // add route change listener for embedded apps
+        if (!delayCssRemoval) {
+            this.#addRouteChangeListener();
+        }
     }
 
     getDecoratedApp = () => {
@@ -47,7 +56,7 @@ export class CssTrackedApp {
                 return newInstance;
             }
 
-            return new CssTrackedApp(newInstance, this.#cssLinkUri, this.#delayCssRemoval).getDecoratedApp();
+            return new CssTrackedApp(newInstance, this.#cssLinkUri, false).getDecoratedApp();
         });
     };
 
@@ -72,6 +81,7 @@ export class CssTrackedApp {
             if (link != null) {
                 this.#decrementOrRemoveCssUsages(link);
             }
+            this.#removeRouteChangeListener();
         }
     };
 
@@ -103,17 +113,29 @@ export class CssTrackedApp {
     #decrementOrRemoveCssUsages(link) {
         const numberOfUsages = this.#getNumberOfLinkUsages(link);
         if (numberOfUsages <= 1) {
-            if (this.#delayCssRemoval) {
-                link.removeAttribute(CssTrackedApp.linkUsagesAttribute);
-                link.setAttribute(CssTrackedApp.markedForRemovalAttribute, 'true');
-            } else {
-                link.remove();
-            }
+            this.#handleLinkRemoval(link);
         } else {
             link.setAttribute(CssTrackedApp.linkUsagesAttribute, (numberOfUsages - 1).toString());
         }
     }
 
+    #handleLinkRemoval(link) {
+        if (this.#shouldDelayRemoval()) {
+            this.#markLinkForRemoval(link);
+        } else {
+            link.remove();
+        }
+    }
+
+    #shouldDelayRemoval() {
+        return this.#delayCssRemoval || this.#isRouteChanged;
+    }
+
+    #markLinkForRemoval(link) {
+        link.removeAttribute(CssTrackedApp.linkUsagesAttribute);
+        link.setAttribute(CssTrackedApp.markedForRemovalAttribute, 'true');
+    }
+
     #getNumberOfLinkUsages(link) {
         const existingValue = link.getAttribute(CssTrackedApp.linkUsagesAttribute);
         return existingValue === null ? 0 : parseInt(existingValue, 10);
@@ -122,4 +144,22 @@ export class CssTrackedApp {
     #findLink() {
         return document.querySelector(`link[href="${this.#cssLinkUri}"]`);
     }
+
+    #handleRouteChange() {
+        this.#isRouteChanged = true;
+    }
+
+    #addRouteChangeListener() {
+        if (!this.#routeChangeListener) {
+            this.#routeChangeListener = this.#handleRouteChange.bind(this);
+            window.addEventListener(ilcEvents.BEFORE_ROUTING, this.#routeChangeListener);
+        }
+    }
+
+    #removeRouteChangeListener() {
+        if (this.#routeChangeListener) {
+            window.removeEventListener(ilcEvents.BEFORE_ROUTING, this.#routeChangeListener);
+            this.#routeChangeListener = null;
+        }
+    }
 }
diff --git a/ilc/client/CssTrackedApps.spec.js b/ilc/client/CssTrackedApps.spec.js
@@ -1,6 +1,7 @@
 import sinon from 'sinon';
 import { expect } from 'chai';
 import { CssTrackedApp } from './CssTrackedApp';
+import ilcEvents from './constants/ilcEvents';
 
 const ilcTestAttributeName = 'data-ilc-test';
 
@@ -206,4 +207,50 @@ describe('CssTrackedApp', function () {
             expect(link.getAttribute(CssTrackedApp.markedForRemovalAttribute)).to.equal('true');
         });
     });
+
+    describe('when embedded app is unmounted', () => {
+        let newApp;
+        let cssLink;
+
+        beforeEach(async () => {
+            const returnValue = Math.random();
+            const appOnCreateNew = createOriginalAppFake(Promise.resolve(returnValue));
+            const originalApp = createOriginalAppFake(Promise.resolve(Math.random()));
+            originalApp.createNew = () => Promise.resolve(appOnCreateNew);
+
+            cssLink = 'data:text/css,<style>div { border: 1px solid blue; }</style>';
+            const cssWrap = new CssTrackedApp(originalApp, cssLink, false).getDecoratedApp();
+            newApp = await cssWrap.createNew();
+        });
+
+        afterEach(() => {
+            sinon.restore();
+        });
+
+        it('should mark link for removal if route change action is in progress', async () => {
+            await newApp.mount();
+
+            let link = document.querySelector(`link[href="${cssLink}"]`);
+            expect(link.getAttribute(CssTrackedApp.linkUsagesAttribute)).to.equal('1');
+
+            window.dispatchEvent(new Event(ilcEvents.BEFORE_ROUTING));
+            await newApp.unmount();
+
+            link = document.querySelector(`link[href="${cssLink}"]`);
+
+            expect(link).to.not.be.null;
+        });
+
+        it('should remove link if embedded app is unmounted without route change', async () => {
+            await newApp.mount();
+
+            let link = document.querySelector(`link[href="${cssLink}"]`);
+            expect(link.getAttribute(CssTrackedApp.linkUsagesAttribute)).to.equal('1');
+
+            await newApp.unmount();
+
+            link = document.querySelector(`link[href="${cssLink}"]`);
+            expect(link).to.be.null;
+        });
+    });
 });