diff --git a/packages/sdk/src/engine/model.ts b/packages/sdk/src/engine/model.ts index 38b46536d..2efae69eb 100644 --- a/packages/sdk/src/engine/model.ts +++ b/packages/sdk/src/engine/model.ts @@ -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); diff --git a/packages/sdk/src/session.subcomp.test.ts b/packages/sdk/src/session.subcomp.test.ts index 91a99e0dd..c52182de6 100644 --- a/packages/sdk/src/session.subcomp.test.ts +++ b/packages/sdk/src/session.subcomp.test.ts @@ -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(` +
inside
+outside
+ `), + ) 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(` +inner
+outer
+inside
+solo
+