fix(sdk): agree removeElement/getElement on duplicate bare ids (#1511)

* fix(sdk): setStyle removes hyphenated properties (was kebab/camel key mismatch)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(sdk): agree removeElement/getElement on duplicate bare ids

A bare hf-id duplicated across a sub-composition element and a top-level
element resolved to different instances: removeElement → resolveScoped →
querySelector (document-order-first, the inner sub-comp dup) while getElement
preferred the canonical match (scopedId === id, the top-level dup). So
removeElement(bareId) removed the inner instance and getElement(bareId) still
found the surviving top-level one — they disagreed.

resolveScoped now resolves an ambiguous BARE id to the canonical (top-level)
instance via isCanonicalScope (walks ancestors for isNewHostBoundary), falling
back to document order when no canonical match exists — matching getElement.
Fully-scoped paths (hf-host/hf-dup) and non-duplicated bare ids are unchanged.

Surfaced by SDK shadow parity (op:delete expected removed, actual present).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-06-16 12:30:54 -07:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 066ea798b4
commit cc055f318d
3 changed files with 126 additions and 9 deletions
+36 -4
View File
@@ -29,25 +29,57 @@ export function parseMutable(html: string): ParsedDocument {
// ─── Element lookup ───────────────────────────────────────────────────────────
export function findById(document: Document, id: string): Element | null {
// CSS.escape is browser-only; hf-ids are restricted identifiers so simple quote-escaping is safe.
const escaped = id.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
return document.querySelector(`[data-hf-id="${escaped}"]`);
// Delegate to resolveScoped so patch replay (undo/redo, override-set apply)
// resolves an id the SAME way forward dispatch does: canonical-first for an
// ambiguous bare id, and scoped-path ("hf-host/hf-leaf") aware. Otherwise the
// two paths disagree on which duplicate a bare id targets and undo reverts the
// wrong element. (function declaration is hoisted.)
return resolveScoped(document, id);
}
function escapeHfId(id: string): string {
return id.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
}
/**
* True when an element lives at the top-level (canonical) scope — i.e. its
* scopedId equals its bare id because no ancestor opens a sub-composition
* boundary. This mirrors document.ts's scopedId construction (childPrefix only
* changes at isNewHostBoundary elements) without rebuilding the snapshot tree.
*/
function isCanonicalScope(el: Element): boolean {
for (let cur = el.parentElement; cur; cur = cur.parentElement) {
if (isNewHostBoundary(cur)) return false;
}
return true;
}
/**
* Resolve a bare or scoped hf-id to its DOM element.
*
* Bare id ("hf-x"): equivalent to findById — top-level document search.
* Bare id ("hf-x"): top-level document search. When the bare id is ambiguous
* (duplicated across a sub-composition and the top level), prefer the canonical
* (top-level) instance — the one whose scopedId equals the bare id — falling
* back to document order when no canonical match exists. This matches
* getElement()'s resolution rule so removeElement / getElement agree on which
* instance an ambiguous bare id targets.
*
* Scoped id ("hf-HOST/hf-LEAF", any depth): each segment narrows the search
* into the subtree of the previous match. This unambiguously addresses an
* element inside a sub-composition even when bare ids collide.
*/
export function resolveScoped(document: Document, id: string): Element | null {
const parts = id.split("/");
// Bare id: prefer the canonical (top-level) match when one exists, so
// resolution agrees with getElement (scopedId === id wins over document order).
if (parts.length === 1) {
const escaped = escapeHfId(id);
const matches = Array.from(document.querySelectorAll(`[data-hf-id="${escaped}"]`));
if (matches.length === 0) return null;
return matches.find((el) => isCanonicalScope(el)) ?? matches[0] ?? null;
}
let context: Element | Document = document;
for (const part of parts) {
const escaped = escapeHfId(part);
+83 -1
View File
@@ -13,7 +13,7 @@
import { describe, it, expect } from "vitest";
import { parseHTML } from "linkedom";
import { ensureHfIds } from "@hyperframes/core/hf-ids";
import { resolveScoped } from "./engine/model.js";
import { resolveScoped, findById } from "./engine/model.js";
import { parseMutable } from "./engine/model.js";
import { buildRoots, flatElements } from "./document.js";
import { openComposition } from "./session.js";
@@ -49,6 +49,24 @@ describe("resolveScoped — flat id", () => {
);
expect(resolveScoped(doc as unknown as Document, "hf-xxxx")).toBeNull();
});
// Regression: findById is the patch-replay/undo resolver. It must agree with
// resolveScoped (forward dispatch) on an ambiguous bare id — both pick the
// canonical (top-level) instance — or undo reverts the wrong duplicate.
it("findById resolves an ambiguous bare id to the canonical instance (== resolveScoped)", () => {
const doc = makeDoc(
inlinedHtml(`
<div data-hf-id="hf-host" data-composition-file="sub.html">
<p data-hf-id="hf-dup" class="inside">inside</p>
</div>
<p data-hf-id="hf-dup" class="outside">outside</p>
`),
) as unknown as Document;
const viaFind = findById(doc, "hf-dup");
const viaResolve = resolveScoped(doc, "hf-dup");
expect(viaFind).toBe(viaResolve);
expect(viaFind?.getAttribute("class")).toBe("outside");
});
});
describe("resolveScoped — scoped id", () => {
@@ -366,6 +384,70 @@ describe("find({ composition })", () => {
});
});
// ─── 5b. Ambiguous bare id: removeElement / getElement agreement ──────────────
describe("ambiguous bare id — removeElement and getElement agree", () => {
// Inner sub-comp dup appears FIRST in document order; the canonical top-level
// dup appears AFTER it. querySelector document-order would return the inner one,
// but getElement prefers the canonical (top-level) match. The two APIs must agree.
const ambiguousHtml = () =>
inlinedHtml(`
<div data-hf-id="hf-root" data-hf-root>
<div data-hf-id="hf-host" data-composition-file="sub.html">
<p data-hf-id="hf-dup" class="inner">inner</p>
</div>
<p data-hf-id="hf-dup" class="outer">outer</p>
</div>
`);
it("bare id resolves to the canonical (top-level) instance, matching getElement", async () => {
const comp = await openComposition(ambiguousHtml());
// getElement prefers the canonical match (scopedId === id) → top-level "outer".
const got = comp.getElement("hf-dup");
expect(got?.scopedId).toBe("hf-dup");
expect(got?.classNames).toContain("outer");
// removeElement(bareId) must remove the SAME instance getElement returned.
comp.removeElement("hf-dup");
// The canonical top-level instance is gone — getElement(bareId) no longer
// finds it (and does NOT silently fall through to the inner sub-comp dup).
expect(comp.getElement("hf-dup")).toBeNull();
// The inner instance survives, addressable only via its fully-scoped path.
const inner = comp.getElement("hf-host/hf-dup");
expect(inner?.classNames).toContain("inner");
});
it("fully-scoped path still targets the inner instance exactly", async () => {
const comp = await openComposition(ambiguousHtml());
comp.removeElement("hf-host/hf-dup");
// Inner gone; canonical top-level survives.
const inner = comp.getElement("hf-host/hf-dup");
expect(inner).toBeNull();
const top = comp.getElement("hf-dup");
expect(top?.scopedId).toBe("hf-dup");
expect(top?.classNames).toContain("outer");
});
it("non-duplicated bare id still resolves and removes normally", async () => {
const html = inlinedHtml(`
<div data-hf-id="hf-root" data-hf-root>
<div data-hf-id="hf-host" data-composition-file="sub.html">
<p data-hf-id="hf-leaf">inside</p>
</div>
<p data-hf-id="hf-solo">solo</p>
</div>
`);
const comp = await openComposition(html);
expect(comp.getElement("hf-solo")?.scopedId).toBe("hf-solo");
comp.removeElement("hf-solo");
expect(comp.getElement("hf-solo")).toBeNull();
});
});
// ─── 6. Scoped id stability across serialize ──────────────────────────────────
describe("scopedId stability across serialize/re-parse", () => {