remove scripts that failed to load (#158)

Co-authored-by: PS <[email protected]>
mjancarik · Apr 28, 2023 · 0cf2f36 · 0cf2f36
1 parent ef1b38f
commit 0cf2f36
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 4 deletions.
diff --git a/packages/integration/src/__tests__/__snapshots__/indexSpec.js.snap b/packages/integration/src/__tests__/__snapshots__/indexSpec.js.snap
@@ -8,6 +8,7 @@ Array [
     "custom-element": "amp-fx-collection",
     "onerror": [Function],
     "onload": [Function],
+    "remove": [MockFunction],
     "removeAttribute": [Function],
     "setAttribute": [Function],
     "src": "http://localhost:4444/static/es11/widget.6541af42bfa3596bb129.js",
@@ -17,6 +18,7 @@ Array [
     "defer": true,
     "onerror": [Function],
     "onload": [Function],
+    "remove": [MockFunction],
     "removeAttribute": [Function],
     "setAttribute": [Function],
     "src": "http://localhost:4444/static/es5/undefined.6961af42bfa3596bb147.js",
@@ -26,6 +28,7 @@ Array [
     "defer": true,
     "onerror": [Function],
     "onload": [Function],
+    "remove": [MockFunction],
     "removeAttribute": [Function],
     "setAttribute": [Function],
     "src": "http://localhost:4444/static/es5/polyfill.6961af42bfa3596bb147.js",
@@ -44,6 +47,7 @@ Array [
         },
       ],
     },
+    "remove": [MockFunction],
     "removeAttribute": [Function],
     "setAttribute": [Function],
     "src": "http://localhost:4444/static/es5/test.6961af42bfa3596bb147.js",
@@ -60,6 +64,7 @@ Array [
     "onerror": [Function],
     "onload": [Function],
     "rel": "stylesheet",
+    "remove": [MockFunction],
     "removeAttribute": [Function],
     "setAttribute": [Function],
     "src": "http://localhost:4444/static/es5/test.6961af42bfa3596bb147.js",
@@ -79,6 +84,7 @@ Array [
         },
       ],
     },
+    "remove": [MockFunction],
     "removeAttribute": [Function],
     "setAttribute": [Function],
     "src": "http://localhost:4444/static/es5/test.6961af42bfa3596bb147.js",

diff --git a/packages/integration/src/__tests__/indexSpec.js b/packages/integration/src/__tests__/indexSpec.js
@@ -15,6 +15,7 @@ describe('Merkur component', () => {
       setAttribute: (name, value) => (fakeAssetObject[name] = value),
       addEventListener: jest.fn((type, callback) => callback(type)),
       src: 'http://localhost:4444/static/es5/test.6961af42bfa3596bb147.js',
+      remove: jest.fn(),
     };
     fakeAssetObjects.push(fakeAssetObject);
 
@@ -191,6 +192,10 @@ describe('Merkur component', () => {
     });
 
     it('should return promise that rejects after script fails to load', (done) => {
+      let script;
+      jest.spyOn(rootElement, 'appendChild').mockImplementation((child) => {
+        script = child;
+      });
       loadScriptAssets(
         [
           {
@@ -209,14 +214,18 @@ describe('Merkur component', () => {
         .catch(() => {
           expect(document.createElement).toHaveBeenCalledTimes(1);
           expect(rootElement.appendChild).toHaveBeenCalledTimes(1);
-
+          expect(script.remove).toHaveBeenCalledTimes(1);
           done();
         });
 
       rejectFakeAssets();
     });
 
     it('should return promise that resolves after script fails to load', (done) => {
+      let script;
+      jest.spyOn(rootElement, 'appendChild').mockImplementation((child) => {
+        script = child;
+      });
       loadScriptAssets(
         [
           {
@@ -233,7 +242,7 @@ describe('Merkur component', () => {
         .then(() => {
           expect(document.createElement).toHaveBeenCalledTimes(1);
           expect(rootElement.appendChild).toHaveBeenCalledTimes(1);
-
+          expect(script.remove).toHaveBeenCalledTimes(1);
           done();
         })
         .catch(() => {
@@ -312,5 +321,40 @@ describe('Merkur component', () => {
           done();
         });
     });
+
+    it('should resolve if script is already present in DOM and loaded', (done) => {
+      jest
+        .spyOn(rootElement, 'querySelector')
+        .mockImplementation(() => 'truthy');
+
+      loadScriptAssets(
+        [
+          {
+            name: 'test.js',
+            type: 'script',
+            source: {
+              es11: 'http://localhost:4444/static/es11/test.6961af42bfa3596bb147.js',
+            },
+            test: 'return true',
+          },
+          {
+            name: 'test2.js',
+            type: 'script',
+            source: {
+              es11: 'http://localhost:4444/static/es11/test.6961af42bfa3596bb147.js',
+            },
+          },
+        ],
+        rootElement
+      )
+        .then(() => {
+          expect(document.createElement).toHaveBeenCalledTimes(0);
+
+          done();
+        })
+        .catch((error) => {
+          done(error);
+        });
+    });
   });
 });
diff --git a/packages/integration/src/index.js b/packages/integration/src/index.js
@@ -5,7 +5,7 @@ function _loadScript(asset, root) {
     const scriptElement = root.querySelector(`script[src='${asset.source}']`);
 
     if (scriptElement) {
-      if (!asset.test) {
+      if (!asset.test || testScript.test(asset.test)) {
         resolve();
       }
 
@@ -22,7 +22,11 @@ function _loadScript(asset, root) {
     if (asset.type === 'script') {
       script.defer = true;
       script.onload = resolve;
-      script.onerror = asset.optional ? resolve : reject;
+      script.onerror = (error) => {
+        script.remove();
+
+        asset.optional ? resolve(error) : reject(error);
+      };
       script.src = asset.source;
 
       const { attr } = asset;