Avoid parsing into DOM when rendering live components

assets/js/phoenix_live_view/rendered.js

@@ -17,6 +17,66 @@ import {
   isCid,
 } from "./utils"
 
+export let modifyRoot = (html, attrs, innerHTML) => {
+  html = html.trimStart()
+  let tagStartsAt = null
+  let pos = 0
+  while(pos < html.length){
+    let maybeStart = html.indexOf("<", pos)
+    if(maybeStart === -1){ break }
+    if(maybeStart >= 0 && html.charAt(maybeStart + 1) !== "!"){
+      tagStartsAt = maybeStart
+      break
+    }
+    pos += html.indexOf("-->", pos)
+  }
+
+  let commentBefore = tagStartsAt === 0 ? null : html.slice(0, tagStartsAt).trim()
+  let contentAfter = null
+  html = html.slice(tagStartsAt).trimStart()
+  let tagNamesEndsAt
+  for(let i = 1; i < html.length; i++){
+    let char = html.charAt(i)
+    if([">", " ", "\n", "\t", "\r"].indexOf(char) >= 0 || (char === "!" && html.charAt(i + 1) === ">")){
+      tagNamesEndsAt = i
+      break
+    }
+  }
+
+  let tag = html.slice(1, tagNamesEndsAt)
+  let tagOpenEndsAt = html.indexOf(">")
+  let tagInnerHTML
+  let closingTag
+  let isVoid = html.charAt(tagOpenEndsAt - 1) === "/"
+  let tagOpenContent = isVoid ? html.slice(0, tagOpenEndsAt - 1) : html.slice(0, tagOpenEndsAt)
+  if(isVoid){
+    contentAfter = html.slice(tagOpenEndsAt + 1) || null
+  } else {
+    closingTag = `</${tag}>`
+    let tagInnerEndsAt = html.lastIndexOf(closingTag)
+    tagInnerHTML = html.slice(tagOpenEndsAt + 1, tagInnerEndsAt)
+    contentAfter = html.slice(tagInnerEndsAt + closingTag.length)
+  }
+
+  let newAttrs = []
+  Object.keys(attrs).forEach(attr => {
+    if(!tagOpenContent.includes(`${attr}="`)){ newAttrs.push([attr, attrs[attr]]) }
+  })
+
+  if(newAttrs.length > 0){
+    tagOpenContent = `${tagOpenContent} ${newAttrs.map(([attr, val]) => `${attr}="${val}"`).join(" ")}`
+  }
+  let closingContent
+  if(isVoid){
+    closingContent = "/>"
+  } else {
+    closingContent = `>${typeof(innerHTML) === "string" ? innerHTML : tagInnerHTML}${closingTag}`
+  }
+  let newHTML = tagOpenContent + closingContent
+  let commentAfter = contentAfter && contentAfter.indexOf("<!--") >= 0 ? contentAfter.trim() : null
+  return [newHTML, commentBefore, commentAfter]
+}
+
 export default class Rendered {
   static extract(diff){
     let {[REPLY]: reply, [EVENTS]: events, [TITLE]: title} = diff
@@ -205,55 +265,14 @@ export default class Rendered {
 
   recursiveCIDToString(components, cid, onlyCids, allowRootComments = true){
     let component = components[cid] || logError(`no component for CID ${cid}`, components)
-    let template = document.createElement("template")
     let [html, streams] = this.recursiveToString(component, components, onlyCids)
-    template.innerHTML = html
-    let container = template.content
     let skip = onlyCids && !onlyCids.has(cid)
+    let attrs = {[PHX_COMPONENT]: cid, id: `${this.parentViewId()}-${cid}`}
+    if(skip){ attrs[PHX_SKIP] = ""}
+    let [newHTML, commentBefore, commentAfter] = modifyRoot(html, attrs, skip ? "" : null)
+    if(allowRootComments){ newHTML = `${commentBefore || ""}${newHTML}${commentAfter || ""}` }
 
-    let [hasChildNodes, hasChildComponents] =
-      Array.from(container.childNodes).reduce(([hasNodes, hasComponents], child, i) => {
-        if(child.nodeType === Node.ELEMENT_NODE){
-          if(child.getAttribute(PHX_COMPONENT)){
-            return [hasNodes, true]
-          }
-          child.setAttribute(PHX_COMPONENT, cid)
-          if(!child.id){ child.id = `${this.parentViewId()}-${cid}-${i}` }
-          if(skip){
-            child.setAttribute(PHX_SKIP, "")
-            child.innerHTML = ""
-          }
-          return [true, hasComponents]
-        } else if(child.nodeType === Node.COMMENT_NODE){
-          // we have to strip root comments when rendering a component directly
-          // for patching because the morphdom target must be exactly the root entrypoint
-          if(!allowRootComments){ child.remove() }
-          return [hasNodes, hasComponents]
-        } else {
-          if(child.nodeValue.trim() !== ""){
-            logError("only HTML element tags are allowed at the root of components.\n\n" +
-              `got: "${child.nodeValue.trim()}"\n\n` +
-              "within:\n", template.innerHTML.trim())
-            child.replaceWith(this.createSpan(child.nodeValue, cid))
-            return [true, hasComponents]
-          } else {
-            child.remove()
-            return [hasNodes, hasComponents]
-          }
-        }
-      }, [false, false])
-
-    if(!hasChildNodes && !hasChildComponents){
-      logError("expected at least one HTML element tag inside a component, but the component is empty:\n",
-        template.innerHTML.trim())
-      return [this.createSpan("", cid).outerHTML, streams]
-    } else if(!hasChildNodes && hasChildComponents){
-      logError("expected at least one HTML element tag directly inside a component, but only subcomponents were found. A component must render at least one HTML tag directly inside itself.",
-        template.innerHTML.trim())
-      return [template.innerHTML, streams]
-    } else {
-      return [template.innerHTML, streams]
-    }
+    return [newHTML, streams]
   }
 
   createSpan(text, cid){

assets/test/modify_root_test.js

@@ -0,0 +1,73 @@
+import {modifyRoot} from "phoenix_live_view/rendered"
+
+describe("modifyRoot stripping comments", () => {
+  test("starting comments", () => {
+    // starting comments
+    let html = `
+<!-- start -->
+<!-- start2 -->
+<div class="px-51"><!-- MENU --><div id="menu">MENU</div></div>
+`
+    let [strippedHTML, commentBefore, commentAfter] = modifyRoot(html, {})
+    expect(strippedHTML).toEqual("<div class=\"px-51\"><!-- MENU --><div id=\"menu\">MENU</div></div>")
+    expect(commentBefore).toEqual(`<!-- start -->
+<!-- start2 -->`)
+    expect(commentAfter).toEqual(null)
+  })
+
+  test("ending comments", () => {
+    let html = `
+<div class="px-52"><!-- MENU --><div id="menu">MENU</div></div>
+<!-- ending -->
+`
+    let [strippedHTML, commentBefore, commentAfter] = modifyRoot(html, {})
+    expect(strippedHTML).toEqual("<div class=\"px-52\"><!-- MENU --><div id=\"menu\">MENU</div></div>")
+    expect(commentBefore).toEqual(null)
+    expect(commentAfter).toEqual("<!-- ending -->")
+  })
+
+  test("staring and ending comments", () => {
+    let html = `
+<!-- starting -->
+<div class="px-53"><!-- MENU --><div id="menu">MENU</div></div>
+<!-- ending -->
+`
+    let [strippedHTML, commentBefore, commentAfter] = modifyRoot(html, {})
+    expect(strippedHTML).toEqual("<div class=\"px-53\"><!-- MENU --><div id=\"menu\">MENU</div></div>")
+    expect(commentBefore).toEqual(`<!-- starting -->`)
+    expect(commentAfter).toEqual(`<!-- ending -->`)
+  })
+  test("merges new attrs", () => {
+    let html = `
+    <div class="px-5"><div id="menu">MENU</div></div>
+    `
+    expect(modifyRoot(html, {id: 123})[0]).toEqual(`<div class="px-5" id="123"><div id="menu">MENU</div></div>`)
+    expect(modifyRoot(html, {id: 123, class: ""})[0]).toEqual(`<div class="px-5" id="123"><div id="menu">MENU</div></div>`)
+    expect(modifyRoot(html, {id: 123, another: ""})[0]).toEqual(`<div class="px-5" id="123" another=""><div id="menu">MENU</div></div>`)
+    // clearing innerHTML
+    expect(modifyRoot(html, {id: 123, another: ""}, "")[0]).toEqual(`<div class="px-5" id="123" another=""></div>`)
+    // self closing
+    let selfClose = `
+    <input class="px-5"/>
+    `
+    expect(modifyRoot(selfClose, {id: 123, another: ""})[0]).toEqual(`<input class="px-5" id="123" another=""/>`)
+  })
+  test("mixed whitespace", () => {
+    let html = `
+    <div
+${"\t"}class="px-5"><div id="menu">MENU</div></div>
+    `
+    expect(modifyRoot(html, {id: 123})[0]).toEqual(`<div
+${"\t"}class="px-5" id="123"><div id="menu">MENU</div></div>`)
+    expect(modifyRoot(html, {id: 123, class: ""})[0]).toEqual(`<div
+${"\t"}class="px-5" id="123"><div id="menu">MENU</div></div>`)
+    expect(modifyRoot(html, {id: 123, another: ""})[0]).toEqual(`<div
+${"\t"}class="px-5" id="123" another=""><div id="menu">MENU</div></div>`)
+    // clearing innerHTML
+    expect(modifyRoot(html, {id: 123, another: ""}, "")[0]).toEqual(`<div
+${"\t"}class="px-5" id="123" another=""></div>`)
+    // self closing
+    let selfClose = `<input${"\t\r\n"}class="px-5"/>`
+    expect(modifyRoot(selfClose, {id: 123, another: ""})[0]).toEqual(`<input${"\t\r\n"}class="px-5" id="123" another=""/>`)
+  })
+})

assets/test/rendered_test.js

@@ -234,12 +234,12 @@ describe("Rendered", () => {
         `<div>
   <p>
     foo
-    <span>0: <b data-phx-component="1" id="123-1-0">FROM index_1 world</b></span><span>1: <b data-phx-component="2" id="123-2-0">FROM index_2 world</b></span>
+    <span>0: <b data-phx-component="1" id="123-1">FROM index_1 world</b></span><span>1: <b data-phx-component="2" id="123-2">FROM index_2 world</b></span>
   </p>
 
   <p>
     bar
-    <span>0: <b data-phx-component="3" id="123-3-0">FROM index_1 world</b></span><span>1: <b data-phx-component="4" id="123-4-0">FROM index_2 world</b></span>
+    <span>0: <b data-phx-component="3" id="123-3">FROM index_1 world</b></span><span>1: <b data-phx-component="4" id="123-4">FROM index_2 world</b></span>
   </p>
 </div>`.trim())
     })

assets/test/view_test.js

@@ -1128,10 +1128,10 @@ describe("View + Component", function(){
     }
 
     view.onJoin({rendered: joinDiff})
-    expect(view.el.innerHTML.trim()).toBe("<div phx-click=\"show-rect\" data-phx-component=\"0\" id=\"container-0-0\">Menu</div><h2>2</h2>")
+    expect(view.el.innerHTML.trim()).toBe("<div phx-click=\"show-rect\" data-phx-component=\"0\" id=\"container-0\">Menu</div><h2>2</h2>")
 
     view.update(updateDiff, [])
-    expect(view.el.innerHTML.trim().replace("\n", "")).toBe("<h1>1</h1><div phx-click=\"show-rect\" data-phx-component=\"0\" id=\"container-0-0\">Menu</div><h2>2</h2>")
+    expect(view.el.innerHTML.trim().replace("\n", "")).toBe("<h1>1</h1><div phx-click=\"show-rect\" data-phx-component=\"0\" id=\"container-0\">Menu</div><h2>2</h2>")
   })
 
   test("respects nested components", () => {
@@ -1151,43 +1151,7 @@ describe("View + Component", function(){
     }
 
     view.onJoin({rendered: joinDiff})
-    expect(view.el.innerHTML.trim()).toBe("<div data-phx-component=\"0\" id=\"container-0-0\">Hello</div><div data-phx-component=\"1\" id=\"container-1-0\">World</div>")
-  })
-
-  test("wraps non-empty text nodes in span tags", () => {
-    let liveSocket = new LiveSocket("/live", Socket)
-    let el = liveViewDOM()
-    let view = simulateJoinedView(el, liveSocket)
-
-    stubChannel(view)
-
-    let joinDiff = {
-      "0": 0,
-      "c": {"0": {"s": ["Hello<div>World</div>\n"]}},
-      "s": ["", ""]
-    }
-
-    jest.spyOn(console, "error").mockImplementation(() => { })
-    view.onJoin({rendered: joinDiff})
-    expect(view.el.innerHTML.trim()).toBe("<span data-phx-component=\"0\"></span><div data-phx-component=\"0\" id=\"container-0-1\">World</div>")
-  })
-
-  test("wraps empty component in a single span tag", () => {
-    let liveSocket = new LiveSocket("/live", Socket)
-    let el = liveViewDOM()
-    let view = simulateJoinedView(el, liveSocket)
-
-    stubChannel(view)
-
-    let joinDiff = {
-      "0": 0,
-      "c": {"0": {"s": ["\n"]}},
-      "s": ["", ""]
-    }
-
-    jest.spyOn(console, "error").mockImplementation(() => { })
-    view.onJoin({rendered: joinDiff})
-    expect(view.el.innerHTML.trim()).toBe("<span data-phx-component=\"0\"></span>")
+    expect(view.el.innerHTML.trim()).toBe("<div data-phx-component=\"0\" id=\"container-0\">Hello</div><div data-phx-component=\"1\" id=\"container-1\">World</div>")
   })
 
   test("destroys children when they are removed by an update", () => {