fix(sdk): address review — live-DOM query cache, single parse, style parse dedup

vanceingalls · claude · vanceingalls · commit edf7306cc298 · 2026-06-11T09:46:50.000-07:00
- getElements/getElement/find now walk the live linkedom DOM via buildRoots
  with a lazily-built cache invalidated on dispatch/applyPatches — no
  serialize→ensureHfIds→parseHTML round trip per query
- openComposition parses once (parseMutable); dropped discarded _doc
  constructor param and the redundant buildDocument call
- document.ts buildElement reuses model.ts getElementStyles — removes
  duplicated parseInlineStyles (also fixes custom-prop camelCase mangling)
- JSDoc note: empty batch() still fires change handlers

Co-Authored-By: Claude Fable 5 &lt;noreply@anthropic.com&gt;
diff --git a/packages/sdk/src/document.ts b/packages/sdk/src/document.ts
@@ -10,6 +10,7 @@
 
 import { parseHTML } from "linkedom";
 import { ensureHfIds } from "@hyperframes/core/hf-ids";
+import { getElementStyles } from "./engine/model.js";
 import type { HyperFramesElement, SdkDocument } from "./types.js";
 
 // Tags that carry no editable content and must not enter the element tree.
@@ -24,22 +25,6 @@ const EXCLUDED_TAGS = new Set([
   "head",
 ]);
 
-// fallow-ignore-next-line complexity
-function parseInlineStyles(styleAttr: string): Record<string, string> {
-  const result: Record<string, string> = {};
-  for (const decl of styleAttr.split(";")) {
-    const idx = decl.indexOf(":");
-    if (idx === -1) continue;
-    const prop = decl.slice(0, idx).trim();
-    const value = decl.slice(idx + 1).trim();
-    if (!prop || !value) continue;
-    // Convert kebab-case → camelCase to match CSSStyleDeclaration convention
-    const camel = prop.replace(/-([a-z])/g, (_, c: string) => c.toUpperCase());
-    result[camel] = value;
-  }
-  return result;
-}
-
 function ownText(el: Element): string | null {
   let text = "";
   el.childNodes.forEach((n) => {
@@ -57,8 +42,7 @@ function buildElement(el: Element): HyperFramesElement | null {
   const id = el.getAttribute("data-hf-id") ?? "";
   if (!id) return null; // should never happen after ensureHfIds, but guard defensively
 
-  const styleAttr = (el as HTMLElement).getAttribute?.("style") ?? "";
-  const inlineStyles = parseInlineStyles(styleAttr);
+  const inlineStyles = getElementStyles(el);
 
   const classAttr = el.getAttribute("class") ?? "";
   const classNames = classAttr
@@ -140,12 +124,28 @@ function extractDuration(doc: Document): number | null {
   return dur ? parseFloat(dur) : null;
 }
 
+/**
+ * Build the element tree from an already-parsed (hf-id-stamped) linkedom Document.
+ * Walks the live DOM directly — no serialize/re-parse round trip. This is what
+ * the session's query API uses against its mutable document.
+ */
+export function buildRoots(document: Document): HyperFramesElement[] {
+  const body = document.body;
+  const roots: HyperFramesElement[] = [];
+  if (body) {
+    for (const child of Array.from(body.children)) {
+      const built = buildElement(child);
+      if (built) roots.push(built);
+    }
+  }
+  return roots;
+}
+
 /**
  * Parse an HTML string into the SDK document model.
  * Calls ensureHfIds first so every element has a stable data-hf-id.
  * Uses linkedom — node-safe (works in agents, CI, server-side).
  */
-// fallow-ignore-next-line complexity
 export function buildDocument(html: string): SdkDocument {
   const stamped = ensureHfIds(html);
 
@@ -155,20 +155,10 @@ export function buildDocument(html: string): SdkDocument {
     ? parseHTML(`<!DOCTYPE html><html><head></head><body>${stamped}</body></html>`)
     : parseHTML(stamped);
 
-  const body = document.body;
-  const roots: HyperFramesElement[] = [];
-
-  if (body) {
-    for (const child of Array.from(body.children)) {
-      const built = buildElement(child);
-      if (built) roots.push(built);
-    }
-  }
-
   const dims = extractDimensions(document);
 
   return {
-    roots,
+    roots: buildRoots(document),
     gsapScript: extractGsapScript(document),
     styles: extractStyles(document),
     width: dims.width,
diff --git a/packages/sdk/src/session.ts b/packages/sdk/src/session.ts
@@ -23,9 +23,8 @@ import type {
   ElementHandle,
 } from "./types.js";
 import { ORIGIN_APPLY_PATCHES, ORIGIN_LOCAL } from "./types.js";
-import { buildDocument, flatElements } from "./document.js";
+import { buildRoots, flatElements } from "./document.js";
 import type { PersistAdapter, PreviewAdapter } from "./adapters/types.js";
-import type { SdkDocument } from "./types.js";
 import { parseMutable } from "./engine/model.js";
 import type { ParsedDocument } from "./engine/model.js";
 import { applyOp, validateOp } from "./engine/mutate.js";
@@ -58,6 +57,9 @@ class CompositionImpl implements Composition {
   /** Accumulated override-set — T3 embedded mode fold contract. */
   private overrides: OverrideSet;
 
+  /** Lazily-built element snapshot, invalidated on every mutation. */
+  private elementsCache: ElementSnapshot[] | null = null;
+
   private currentSelection: string[] = [];
 
   private changeHandlers: Array<() => void> = [];
@@ -76,7 +78,7 @@ class CompositionImpl implements Composition {
   private batchOpTypes: string[] = [];
   private batchOrigin: unknown = ORIGIN_LOCAL;
 
-  constructor(_doc: SdkDocument, parsed: ParsedDocument, opts: OpenCompositionOptions) {
+  constructor(parsed: ParsedDocument, opts: OpenCompositionOptions) {
     this.parsed = parsed;
     this.persist = opts.persist;
     this.preview = opts.preview;
@@ -143,7 +145,9 @@ class CompositionImpl implements Composition {
   // ── Query API (F1) ───────────────────────────────────────────────────────────
 
   getElements(): ElementSnapshot[] {
-    return flatElements(buildDocument(serializeDocument(this.parsed)).roots);
+    // Walk the live linkedom DOM directly — no serialize/re-parse round trip.
+    this.elementsCache ??= flatElements(buildRoots(this.parsed.document));
+    return [...this.elementsCache];
   }
 
   getElement(id: HfId): ElementSnapshot | null {
@@ -209,6 +213,8 @@ class CompositionImpl implements Composition {
       return;
     }
 
+    this.elementsCache = null;
+
     // Update override-set from forward patches
     for (const p of forward) {
       const key = pathToKey(p.path);
@@ -229,6 +235,12 @@ class CompositionImpl implements Composition {
     }
   }
 
+  /**
+   * Coalesce multiple dispatches into one undo entry / one patch event.
+   * Note: a batch that produces no effective mutations still fires 'change'
+   * handlers (parity with no-op dispatch) — subscribers must not assume
+   * silence when wrapping speculative operations.
+   */
   // fallow-ignore-next-line complexity
   batch(fn: () => void, opts?: { origin?: unknown }): void {
     const origin = opts?.origin ?? ORIGIN_LOCAL;
@@ -331,6 +343,7 @@ class CompositionImpl implements Composition {
     // For applyPatches the "inverse" is the current state before application.
     // Emit a patch event so history and subscribers stay in sync.
     applyPatchesToDocument(this.parsed, patches);
+    this.elementsCache = null;
 
     // Update override-set
     for (const p of patches) {
@@ -373,9 +386,10 @@ export async function openComposition(
   html: string,
   opts?: OpenCompositionOptions,
 ): Promise<Composition> {
-  const doc = buildDocument(html);
+  // Single parse: parseMutable stamps hf-ids + builds the live linkedom DOM;
+  // the query API derives element snapshots from it lazily.
   const parsed = parseMutable(html);
-  const session = new CompositionImpl(doc, parsed, opts ?? {});
+  const session = new CompositionImpl(parsed, opts ?? {});
 
   const isEmbedded = opts?.overrides !== undefined;
 
