fix: removeDomNodes with HTMLCollection (#583)

Also converted to static helper instead of scoped function for testability.
vaadin · Mar 12, 2021 · 216c126 · 216c126
1 parent f95dc4f
commit 216c126
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 13 deletions.
diff --git a/src/router.js b/src/router.js
@@ -101,15 +101,6 @@ function processNewChildren(newChildren, route) {
   }
 }
 
-function removeDomNodes(nodes) {
-  if (nodes && nodes.length) {
-    const parent = nodes[0].parentNode;
-    for (let i = 0; i < nodes.length; i++) {
-      parent.removeChild(nodes[i]);
-    }
-  }
-}
-
 function getPathnameForRouter(pathname, router) {
   const base = router.__effectiveBaseUrl;
   return base
@@ -504,7 +495,7 @@ export class Router extends Resolver {
           if (shouldUpdateHistory) {
             this.__updateBrowserHistory(context);
           }
-          removeDomNodes(this.__outlet && this.__outlet.children);
+          Router.__removeDomNodes(this.__outlet && this.__outlet.children);
           this.location = createLocation(Object.assign(context, {resolver: this}));
           fireRouterEvent('error', Object.assign({router: this, error}, context));
           throw error;
@@ -809,15 +800,15 @@ export class Router extends Resolver {
 
   __removeDisappearingContent() {
     if (this.__disappearingContent) {
-      removeDomNodes(this.__disappearingContent);
+      Router.__removeDomNodes(this.__disappearingContent);
     }
     this.__disappearingContent = null;
     this.__appearingContent = null;
   }
 
   __removeAppearingContent() {
     if (this.__disappearingContent && this.__appearingContent) {
-      removeDomNodes(this.__appearingContent);
+      Router.__removeDomNodes(this.__appearingContent);
       this.__disappearingContent = null;
       this.__appearingContent = null;
     }
@@ -845,7 +836,7 @@ export class Router extends Resolver {
           currentComponent);
       } finally {
         if (this.__disappearingContent.indexOf(currentComponent) > -1) {
-          removeDomNodes(currentComponent.children);
+          Router.__removeDomNodes(currentComponent.children);
         }
       }
     }
@@ -1002,4 +993,15 @@ export class Router extends Resolver {
       : path;
     return fireRouterEvent('go', {pathname, search, hash});
   }
+
+  static __removeDomNodes(nodes) {
+    if (nodes && nodes.length) {
+      const parent = nodes[0].parentNode;
+      const childrenCount = nodes.length - 1;
+
+      for (let i = childrenCount; i >= 0; i--) {
+        parent.removeChild(nodes[i]);
+      }
+    }
+  }
 }
diff --git a/test/router/router.spec.html b/test/router/router.spec.html
@@ -2239,6 +2239,28 @@
             expect(registrations[0]).to.have.property('version').that.is.a.string;
           });
         });
+
+        describe('__removeDomNodes (function)', () => {
+          let parent, childA, childB = [];
+
+          beforeEach(() => {
+            parent = document.createElement('div');
+            childA = document.createElement('div');
+            childB = document.createElement('div');
+            parent.appendChild(childA);
+            parent.appendChild(childB);
+          });
+
+          it('should remove all nodes when passed HTMLCollection', async() => {
+            Vaadin.Router.__removeDomNodes(parent.children);
+            expect(parent.children).to.have.lengthOf(0);
+          });
+
+          it('should remove all nodes when passed JS array', async() => {
+            Vaadin.Router.__removeDomNodes([childA, childB]);
+            expect(parent.children).to.have.lengthOf(0);
+          });
+        });
       });
     });