Repository: single-spa/single-spa-layout
Pull request #23 (2020-05-19 00:05:15)

Layout engine fixes and support for raw text nodes
- Avoid traversing child routes when we know the parent route is being unmounted
- Support appending raw text nodes to the dom
- Support appending comment nodes


Patch:
diff --git a/src/constructLayoutEngine.js b/src/constructLayoutEngine.js
index ae554bc..d6003b7 100644
--- a/src/constructLayoutEngine.js
+++ b/src/constructLayoutEngine.js
@@ -168,15 +168,17 @@ function recurseRoutes({
           route.connectedNode = newNode;
         }
 
-        recurseRoutes({
-          path,
-          pathMatch,
-          routes: route.routes,
-          parentContainer: route.connectedNode,
-          previousSibling: null,
-          shouldMount,
-          pendingRemovals,
-        });
+        if (route.routes) {
+          recurseRoutes({
+            path,
+            pathMatch,
+            routes: route.routes,
+            parentContainer: route.connectedNode,
+            previousSibling: null,
+            shouldMount,
+            pendingRemovals,
+          });
+        }
 
         previousSibling = route.connectedNode;
       } else {
@@ -199,9 +201,9 @@ function recurseRoutes({
  * @param {Node=} previousSibling
  */
 function insertNode(node, container, previousSibling) {
-  if (previousSibling) {
+  if (previousSibling && previousSibling.nextSibling) {
     // move to be immediately after previousSibling
-    previousSibling.insertAdjacentElement("afterend", node);
+    previousSibling.parentNode.insertBefore(node, previousSibling.nextSibling);
   } else if (node.parentNode !== container) {
     // append to end of the container
     container.appendChild(node);


Test patch:
diff --git a/test/browser-only/__snapshots__/constructLayoutEngine-browser.test.js.snap b/test/browser-only/__snapshots__/constructLayoutEngine-browser.test.js.snap
index 6646c86..85f40b8 100644
--- a/test/browser-only/__snapshots__/constructLayoutEngine-browser.test.js.snap
+++ b/test/browser-only/__snapshots__/constructLayoutEngine-browser.test.js.snap
@@ -1,5 +1,79 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`constructLayoutEngine browser can process the dom elements fixture 1`] = `
+<body>
+  <div
+    id="single-spa-application:header"
+  />
+  <div
+    class="main-content"
+  />
+  <span>
+    <!-- The footer -->
+    <footer>
+      <a
+        href="/about"
+      >
+        About
+      </a>
+    </footer>
+  </span>
+</body>
+`;
+
+exports[`constructLayoutEngine browser can process the dom elements fixture 2`] = `
+<body>
+  <div
+    id="single-spa-application:header"
+  />
+  <div
+    class="main-content"
+  >
+    <button>
+      Top level content
+    </button>
+    <div
+      id="single-spa-application:app1"
+    />
+    <!-- Comments and raw text should work -->
+    
+          Some raw text
+        
+  </div>
+  <span>
+    <!-- The footer -->
+    <footer>
+      <a
+        href="/about"
+      >
+        About
+      </a>
+    </footer>
+  </span>
+</body>
+`;
+
+exports[`constructLayoutEngine browser can process the dom elements fixture 3`] = `
+<body>
+  <div
+    id="single-spa-application:header"
+  />
+  <div
+    class="main-content"
+  />
+  <span>
+    <!-- The footer -->
+    <footer>
+      <a
+        href="/about"
+      >
+        About
+      </a>
+    </footer>
+  </span>
+</body>
+`;
+
 exports[`constructLayoutEngine browser can process the layout in the medium.html fixture 1`] = `
 <body>
   <div
diff --git a/test/browser-only/constructLayoutEngine-browser.test.js b/test/browser-only/constructLayoutEngine-browser.test.js
index b59cfa5..f37b4f2 100644
--- a/test/browser-only/constructLayoutEngine-browser.test.js
+++ b/test/browser-only/constructLayoutEngine-browser.test.js
@@ -11,6 +11,7 @@ describe(`constructLayoutEngine browser`, () => {
   let layoutEngine;
 
   afterEach(() => {
+    history.pushState(history.state, document.title, "/");
     document.body.innerHTML = "";
     if (layoutEngine) {
       layoutEngine.deactivate();
@@ -334,7 +335,7 @@ describe(`constructLayoutEngine browser`, () => {
     const loadApp = jest.fn();
     const routes = constructRoutes(routerElement);
     const applications = constructApplications({ routes, loadApp });
-    const layoutEngine = constructLayoutEngine({ routes, applications });
+    layoutEngine = constructLayoutEngine({ routes, applications });
 
     expect(document.querySelector("body")).toMatchSnapshot();
 
@@ -447,4 +448,65 @@ describe(`constructLayoutEngine browser`, () => {
     );
     expect(document.querySelector("body")).toMatchSnapshot();
   });
+
+  it(`can process the dom elements fixture`, () => {
+    const { routerElement } = parseFixture("dom-elements.html");
+    const loadApp = jest.fn();
+    const routes = constructRoutes(routerElement);
+    const applications = constructApplications({ routes, loadApp });
+    layoutEngine = constructLayoutEngine({ routes, applications });
+
+    window.dispatchEvent(
+      new CustomEvent("single-spa:before-mount-routing-event")
+    );
+    window.dispatchEvent(
+      new CustomEvent("single-spa:app-change", {
+        detail: {
+          appsByNewStatus: {
+            MOUNTED: ["header"],
+            NOT_MOUNTED: [],
+            NOT_LOADED: [],
+          },
+        },
+      })
+    );
+
+    expect(document.querySelector("body")).toMatchSnapshot();
+
+    // transition to /app1 route
+    history.pushState(history.state, document.title, "/app1");
+    window.dispatchEvent(
+      new CustomEvent("single-spa:before-mount-routing-event")
+    );
+    window.dispatchEvent(
+      new CustomEvent("single-spa:app-change", {
+        detail: {
+          appsByNewStatus: {
+            MOUNTED: ["app1"],
+            NOT_MOUNTED: [],
+            NOT_LOADED: [],
+          },
+        },
+      })
+    );
+    expect(document.querySelector("body")).toMatchSnapshot();
+
+    // transition to / route
+    history.pushState(history.state, document.title, "/");
+    window.dispatchEvent(
+      new CustomEvent("single-spa:before-mount-routing-event")
+    );
+    window.dispatchEvent(
+      new CustomEvent("single-spa:app-change", {
+        detail: {
+          appsByNewStatus: {
+            MOUNTED: [],
+            NOT_MOUNTED: ["app1"],
+            NOT_LOADED: [],
+          },
+        },
+      })
+    );
+    expect(document.querySelector("body")).toMatchSnapshot();
+  });
 });
diff --git a/test/fixtures/dom-elements.html b/test/fixtures/dom-elements.html
index ca9f296..0d3b927 100644
--- a/test/fixtures/dom-elements.html
+++ b/test/fixtures/dom-elements.html
@@ -12,8 +12,16 @@
         <route path="app1">
           <button>Top level content</button>
           <application name="app1"></application>
+          <!-- Comments and raw text should work -->
+          Some raw text
         </route>
       </div>
+      <span>
+        <!-- The footer -->
+        <footer>
+          <a href="/about">About</a>
+        </footer>
+      </span>
     </single-spa-router>
   </body>
 </html>