no wrapper span (#1439)

Author: mbostock

package.json

@@ -68,6 +68,7 @@
     "d3-hierarchy": "^3.1.2",
     "esbuild": "^0.20.1",
     "fast-array-diff": "^1.1.0",
+    "fast-deep-equal": "^3.1.3",
     "gray-matter": "^4.0.3",
     "he": "^1.2.0",
     "highlight.js": "^11.8.0",
@@ -118,7 +119,6 @@
     "eslint-config-prettier": "^9.1.0",
     "eslint-import-resolver-typescript": "^3.6.1",
     "eslint-plugin-import": "^2.29.0",
-    "fast-deep-equal": "^3.1.3",
     "glob": "^10.3.10",
     "mocha": "^10.2.0",
     "prettier": "^3.0.3 <3.1",

src/client/main.js

@@ -20,17 +20,19 @@ export const runtime = new Runtime(library);
 export const main = runtime.module();
 
 const cellsById = new Map();
+const rootsById = findRoots(document.body);
 
 export function define(cell) {
   const {id, inline, inputs = [], outputs = [], body} = cell;
   const variables = [];
-  cellsById.get(id)?.variables.forEach((v) => v.delete());
   cellsById.set(id, {cell, variables});
-  const root = document.querySelector(`#cell-${id}`);
-  const loading = root.querySelector(".observablehq-loading");
+  const root = rootsById.get(id);
+  const loading = findLoading(root);
+  root._nodes = [];
+  if (loading) root._nodes.push(loading);
   const pending = () => reset(root, loading);
   const rejected = (error) => reject(root, error);
-  const v = main.variable({_node: root, pending, rejected}, {shadow: {}}); // _node for visibility promise
+  const v = main.variable({_node: root.parentNode, pending, rejected}, {shadow: {}}); // _node for visibility promise
   if (inputs.includes("display") || inputs.includes("view")) {
     let displayVersion = -1; // the variable._version of currently-displayed values
     const display = inline ? displayInline : displayBlock;
@@ -67,39 +69,91 @@ export function define(cell) {
 // original loading indicator in this case, which applies to inline expressions
 // and expression code blocks.
 function reset(root, loading) {
-  if (root.classList.contains("observablehq--error")) {
-    root.classList.remove("observablehq--error");
+  if (root._error) {
+    root._error = false;
     clear(root);
-    if (loading) root.append(loading);
+    if (loading) displayNode(root, loading);
   }
 }
 
 function reject(root, error) {
   console.error(error);
-  root.classList.add("observablehq--error"); // see reset
+  root._error = true; // see reset
   clear(root);
-  root.append(inspectError(error));
+  displayNode(root, inspectError(error));
+}
+
+function displayNode(root, node) {
+  if (node.nodeType === 11) {
+    let child;
+    while ((child = node.firstChild)) {
+      root._nodes.push(child);
+      root.parentNode.insertBefore(child, root);
+    }
+  } else {
+    root._nodes.push(node);
+    root.parentNode.insertBefore(node, root);
+  }
 }
 
 function clear(root) {
-  root.textContent = "";
+  for (const v of root._nodes) v.remove();
+  root._nodes.length = 0;
 }
 
 function displayInline(root, value) {
-  if (isNode(value) || typeof value === "string" || !value?.[Symbol.iterator]) root.append(value);
-  else root.append(...value);
+  if (isNode(value)) {
+    displayNode(root, value);
+  } else if (typeof value === "string" || !value?.[Symbol.iterator]) {
+    displayNode(root, document.createTextNode(value));
+  } else {
+    for (const v of value) {
+      displayNode(root, isNode(v) ? v : document.createTextNode(v));
+    }
+  }
 }
 
 function displayBlock(root, value) {
-  root.append(isNode(value) ? value : inspect(value));
+  displayNode(root, isNode(value) ? value : inspect(value));
 }
 
 export function undefine(id) {
-  cellsById.get(id)?.variables.forEach((v) => v.delete());
+  clear(rootsById.get(id));
+  cellsById.get(id).variables.forEach((v) => v.delete());
   cellsById.delete(id);
 }
 
 // Note: Element.prototype is instanceof Node, but cannot be inserted!
 function isNode(value) {
   return value instanceof Node && value instanceof value.constructor;
 }
+
+export function findRoots(root) {
+  const roots = new Map();
+  const iterator = document.createNodeIterator(root, 128, null);
+  let node;
+  while ((node = iterator.nextNode())) {
+    if (isRoot(node)) {
+      roots.set(node.data.slice(1, -1), node);
+    }
+  }
+  return roots;
+}
+
+function isRoot(node) {
+  return node.nodeType === 8 && /^:[0-9a-f]{8}(?:-\d+)?:$/.test(node.data);
+}
+
+function isLoading(node) {
+  return node.nodeType === 1 && node.tagName === "OBSERVABLEHQ-LOADING";
+}
+
+export function findLoading(root) {
+  const sibling = root.previousSibling;
+  return sibling && isLoading(sibling) ? sibling : null;
+}
+
+export function registerRoot(id, node) {
+  if (node == null) rootsById.delete(id);
+  else rootsById.set(id, node);
+}

src/client/preview.js

@@ -1,5 +1,6 @@
 import {FileAttachment, registerFile} from "npm:@observablehq/stdlib";
 import {main, runtime, undefine} from "./main.js";
+import {findLoading, findRoots, registerRoot} from "./main.js";
 import {enableCopyButtons} from "./pre.js";
 
 export * from "./index.js";
@@ -48,12 +49,30 @@ export function open({hash, eval: compile} = {}) {
             case "add": {
               for (const item of items) {
                 const pos = oldPos + offset;
-                if (pos < root.children.length) {
-                  root.children[pos].insertAdjacentHTML("beforebegin", item);
+                if (pos < root.childNodes.length) {
+                  const child = root.childNodes[pos];
+                  if (item.type === 1) {
+                    if (child.nodeType === 1) {
+                      child.insertAdjacentHTML("beforebegin", item.value);
+                    } else {
+                      root.insertAdjacentHTML("beforeend", item.value);
+                      root.insertBefore(root.lastChild, child);
+                    }
+                  } else if (item.type === 3) {
+                    root.insertBefore(document.createTextNode(item.value), child);
+                  } else if (item.type === 8) {
+                    root.insertBefore(document.createComment(item.value), child);
+                  }
                 } else {
-                  root.insertAdjacentHTML("beforeend", item);
+                  if (item.type === 1) {
+                    root.insertAdjacentHTML("beforeend", item.value);
+                  } else if (item.type === 3) {
+                    root.appendChild(document.createTextNode(item.value));
+                  } else if (item.type === 8) {
+                    root.appendChild(document.createComment(item.value));
+                  }
                 }
-                indexCells(addedCells, root.children[pos]);
+                indexCells(addedCells, root.childNodes[pos]);
                 ++offset;
               }
               break;
@@ -62,26 +81,35 @@ export function open({hash, eval: compile} = {}) {
               let removes = 0;
               for (let i = 0; i < items.length; ++i) {
                 const pos = oldPos + offset;
-                if (pos < root.children.length) {
-                  const child = root.children[pos];
+                if (pos < root.childNodes.length) {
+                  const child = root.childNodes[pos];
                   indexCells(removedCells, child);
                   child.remove();
                   ++removes;
                 } else {
-                  console.error(`remove out of range: ${pos} ≮ ${root.children.length}`);
+                  console.error(`remove out of range: ${pos} ≮ ${root.childNodes.length}`);
                 }
               }
               offset -= removes;
               break;
             }
           }
         }
-        for (const [id, removed] of removedCells) {
-          addedCells.get(id)?.replaceWith(removed);
-        }
         for (const id of message.code.removed) {
           undefine(id);
         }
+        for (const [id, removed] of removedCells) {
+          if (!addedCells.has(id)) {
+            registerRoot(id, null);
+          } else {
+            replaceRoot(addedCells.get(id), removed);
+          }
+        }
+        for (const [id, root] of addedCells) {
+          if (!removedCells.has(id)) {
+            registerRoot(id, root);
+          }
+        }
         for (const body of message.code.added) {
           compile(body);
         }
@@ -138,11 +166,8 @@ export function open({hash, eval: compile} = {}) {
   };
 
   function indexCells(map, node) {
-    if (node.id.startsWith("cell-")) {
-      map.set(node.id, node);
-    }
-    for (const cell of node.querySelectorAll("[id^=cell-]")) {
-      map.set(cell.id, cell);
+    for (const [id, root] of findRoots(node)) {
+      map.set(id, root);
     }
   }
 
@@ -151,3 +176,11 @@ export function open({hash, eval: compile} = {}) {
     socket.send(JSON.stringify(message));
   }
 }
+
+export function replaceRoot(added, removed) {
+  findLoading(added)?.remove();
+  added.replaceWith(removed);
+  for (const n of removed._nodes) {
+    removed.parentNode.insertBefore(n, removed);
+  }
+}

src/html.ts

@@ -169,6 +169,8 @@ export function rewriteHtml(
         ? hljs.highlight(child.textContent!, {language}).value
         : isElement(child)
         ? child.outerHTML
+        : isComment(child)
+        ? `<!--${he.escape(child.data)}-->`
         : "";
     }
     code.innerHTML = html;
@@ -183,6 +185,29 @@ export function rewriteHtml(
     h.append(a);
   }
 
+  // For incremental update during preview, we need to know the direct children
+  // of the body statically; therefore we must wrap any top-level cells with a
+  // span to avoid polluting the direct children with dynamic content.
+  for (let child = document.body.firstChild; child; child = child.nextSibling) {
+    if (isRoot(child)) {
+      const parent = document.createElement("span");
+      const loading = findLoading(child);
+      child.replaceWith(parent);
+      if (loading) parent.appendChild(loading);
+      parent.appendChild(child);
+      child = parent;
+    }
+  }
+
+  // In some contexts, such as a table, the <observablehq-loading> element may
+  // be reparented; enforce the requirement that the <observablehq-loading>
+  // element immediately precedes its root by removing any violating elements.
+  for (const l of document.querySelectorAll("observablehq-loading")) {
+    if (!l.nextSibling || !isRoot(l.nextSibling)) {
+      l.remove();
+    }
+  }
+
   return document.body.innerHTML;
 }
 
@@ -208,14 +233,31 @@ function resolveSrcset(srcset: string, resolve: (specifier: string) => string):
     .join(", ");
 }
 
-function isText(node: Node): node is Text {
+export function isText(node: Node): node is Text {
   return node.nodeType === 3;
 }
 
-function isElement(node: Node): node is Element {
+export function isComment(node: Node): node is Comment {
+  return node.nodeType === 8;
+}
+
+export function isElement(node: Node): node is Element {
   return node.nodeType === 1;
 }
 
+function isRoot(node: Node): node is Comment {
+  return isComment(node) && /^:[0-9a-f]{8}(?:-\d+)?:$/.test(node.data);
+}
+
+function isLoading(node: Node): node is Element {
+  return isElement(node) && node.tagName === "OBSERVABLEHQ-LOADING";
+}
+
+function findLoading(node: Node): Element | null {
+  const sibling = node.previousSibling;
+  return sibling && isLoading(sibling) ? sibling : null;
+}
+
 /**
  * Denotes a string that contains HTML source; when interpolated into an html
  * tagged template literal, it will not be escaped. Use Html.unsafe to denote

src/markdown.ts

@@ -106,9 +106,9 @@ function makeFenceRenderer(baseRenderer: RenderRule): RenderRule {
         // TODO const sourceLine = context.startLine + context.currentLine;
         const node = parseJavaScript(source, {path});
         context.code.push({id, node});
-        html += `<div id="cell-${id}" class="observablehq observablehq--block">${
-          node.expression ? '<span class="observablehq-loading"></span>' : ""
-        }</div>\n`;
+        html += `<div class="observablehq observablehq--block">${
+          node.expression ? "<observablehq-loading></observablehq-loading>" : ""
+        }<!--:${id}:--></div>\n`;
       }
     } catch (error) {
       if (!(error instanceof SyntaxError)) throw error;
@@ -261,14 +261,12 @@ function makePlaceholderRenderer(): RenderRule {
       // TODO sourceLine: context.startLine + context.currentLine
       const node = parseJavaScript(token.content, {path, inline: true});
       context.code.push({id, node});
-      return `<span id="cell-${id}"><span class="observablehq-loading"></span></span>`;
+      return `<observablehq-loading></observablehq-loading><!--:${id}:-->`;
     } catch (error) {
       if (!(error instanceof SyntaxError)) throw error;
-      return `<span id="cell-${id}">
-  <span class="observablehq--inspect observablehq--error" style="display: block;">SyntaxError: ${he.escape(
-    error.message
-  )}</span>
-</span>`;
+      return `<span class="observablehq--inspect observablehq--error" style="display: block;">SyntaxError: ${he.escape(
+        error.message
+      )}</span>`;
     }
   };
 }