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

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-06-16 12:30:35 -07:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 4b4a3eb63d
commit 066ea798b4
4 changed files with 105 additions and 21 deletions
+6 -3
View File
@@ -88,7 +88,7 @@ export function findRoot(document: Document): Element | null {
// ─── Inline style helpers ─────────────────────────────────────────────────────
function toCamel(prop: string): string {
export function toCamel(prop: string): string {
if (prop.startsWith("--")) return prop;
return prop.replace(/-([a-z])/g, (_, c: string) => (c as string).toUpperCase());
}
@@ -127,10 +127,13 @@ export function getElementStyles(el: Element): Record<string, string> {
export function setElementStyles(el: Element, updates: Record<string, string | null>): void {
const current = getElementStyles(el);
for (const [prop, value] of Object.entries(updates)) {
// Stored map is keyed camelCase (parseStyleAttr); custom props (--foo) stay
// verbatim. Normalize the incoming key the same way for both set and delete.
const key = toCamel(prop);
if (value === null) {
delete current[prop];
delete current[key];
} else {
current[prop] = value;
current[key] = value;
}
}
const serialized = serializeStyleAttr(current);
+81 -1
View File
@@ -7,7 +7,7 @@
*/
import { describe, it, expect } from "vitest";
import { parseMutable } from "./model.js";
import { parseMutable, getElementStyles, setElementStyles } from "./model.js";
import { applyOp, validateOp } from "./mutate.js";
import { applyPatchesToDocument } from "./apply-patches.js";
import { pathToKey } from "./patches.js";
@@ -106,6 +106,42 @@ describe("setStyle", () => {
const key = pathToKey("/elements/hf-title/inlineStyles/fontSize");
expect(key).toBe("hf-title.style.fontSize");
});
// Regression: a HYPHENATED (kebab) style key must derive its inverse against
// the camelCase-keyed store, or oldValue is null → undo deletes the prior
// value instead of restoring it, and a removal skips the inverse entirely.
it("derives correct inverse for a kebab style key (change)", () => {
const parsed = fresh();
const result = applyOp(parsed, {
type: "setStyle",
target: "hf-title",
styles: { "font-size": "96px" },
});
// inverse restores the prior 64px (replace), not a remove
expect(result.inverse[0]).toEqual({
op: "replace",
path: "/elements/hf-title/inlineStyles/fontSize",
value: "64px",
});
});
it("emits the inverse for a kebab style removal (no DOM/patch desync)", () => {
const parsed = fresh();
const result = applyOp(parsed, {
type: "setStyle",
target: "hf-title",
styles: { "font-size": null },
});
// removal must be recorded (forward remove + inverse add restoring 64px)
expect(result.forward[0]?.op).toBe("remove");
expect(result.inverse[0]).toEqual({
op: "add",
path: "/elements/hf-title/inlineStyles/fontSize",
value: "64px",
});
const el = parsed.document.querySelector('[data-hf-id="hf-title"]');
expect(el?.getAttribute("style") ?? "").not.toContain("font-size");
});
});
// ─── setText ─────────────────────────────────────────────────────────────────
@@ -286,6 +322,50 @@ describe("removeElement", () => {
});
});
// ─── setElementStyles (model helper) ──────────────────────────────────────────
describe("setElementStyles key normalization", () => {
function elWith(style: string): Element {
const parsed = parseMutable(`<div data-hf-id="hf-x" data-hf-root style="${style}"></div>`);
const el = parsed.document.querySelector('[data-hf-id="hf-x"]');
if (!el) throw new Error("fixture element missing");
return el;
}
it("removes a hyphenated property when value is null", () => {
const el = elWith("transform-origin: center center; opacity: 0.5");
setElementStyles(el, { "transform-origin": null });
const styles = getElementStyles(el);
expect(styles.transformOrigin).toBeUndefined();
expect(el.getAttribute("style")).not.toContain("transform-origin");
// sibling prop untouched
expect(styles.opacity).toBe("0.5");
});
it("removes a CSS custom property when value is null", () => {
const el = elWith("--brand-color: #f00; opacity: 0.5");
setElementStyles(el, { "--brand-color": null });
const styles = getElementStyles(el);
expect(styles["--brand-color"]).toBeUndefined();
expect(el.getAttribute("style")).not.toContain("--brand-color");
expect(styles.opacity).toBe("0.5");
});
it("sets a camelCase property and keeps existing props", () => {
const el = elWith("color: #fff");
setElementStyles(el, { fontSize: "96px" });
const styles = getElementStyles(el);
expect(styles.fontSize).toBe("96px");
expect(styles.color).toBe("#fff");
});
it("sets a hyphenated property under its camelCase key", () => {
const el = elWith("");
setElementStyles(el, { "transform-origin": "top left" });
expect(getElementStyles(el).transformOrigin).toBe("top left");
});
});
// ─── setVariableValue ─────────────────────────────────────────────────────────
describe("setVariableValue", () => {
+9 -2
View File
@@ -14,6 +14,7 @@ import {
findRoot,
getElementStyles,
setElementStyles,
toCamel,
getOwnText,
setOwnText,
getSiblingIndex,
@@ -199,8 +200,14 @@ function handleSetStyle(
const old = getElementStyles(el);
setElementStyles(el, styles);
for (const [prop, value] of Object.entries(styles)) {
const path = stylePath(id, prop);
const oldValue = old[prop] ?? null;
// Normalize to the camelCase key the style map + patch grammar use. A
// hyphenated op key ("transform-origin") otherwise misses the camelCase
// store, so oldValue is always null → undo deletes/loses the prior value,
// a removal skips its inverse patch entirely (DOM/patch-log desync), and
// the patch path/override-set key diverge from the camelCase grammar.
const key = toCamel(prop);
const path = stylePath(id, key);
const oldValue = old[key] ?? null;
if (value !== null) {
const p = scalarChange(path, oldValue, value);
result.forward.push(p.forward);
+9 -15
View File
@@ -181,32 +181,26 @@ describe("sdkShadowDispatch (integration)", () => {
// Fix 3 verdict (REAL DIVERGENCE, not a readback artifact): the inline-style
// read-back already reads only the AUTHORED style attribute (getElementStyles →
// parseStyleAttr), never computed styles. The transform-origin event
// (expected null actual "center center") is a genuine SDK bug: setStyle removal
// of a HYPHENATED property silently no-ops because setElementStyles deletes the
// kebab key while the style map is keyed camelCase. The shadow CORRECTLY flags
// it; the fix belongs in the SDK (packages/sdk/src/engine/model.ts), not here.
it("CORRECTLY flags the SDK transform-origin removal no-op (real divergence)", async () => {
// parseStyleAttr), never computed styles. The transform-origin divergence
// (expected null actual "center center") was a genuine SDK bug setStyle
// removal of a HYPHENATED property silently no-opped because setElementStyles
// deleted the kebab key while the style map is keyed camelCase. Now FIXED in
// the SDK (model.ts setElementStyles normalizes the key via toCamel), so the
// shadow sees parity: removal applies and there is no mismatch.
it("reports clean removal of a hyphenated style (SDK setStyle kebab/camel fix)", async () => {
const { sdkShadowDispatch } = await import("./sdkShadow");
const TO_HTML = /* html */ `<!DOCTYPE html>
<html><body><div data-hf-id="hf-box" style="transform-origin: center center">x</div></body></html>`;
const session = await openComposition(TO_HTML);
// op intends to REMOVE transform-origin (value null) ...
const ops: PatchOperation[] = [
{ type: "inline-style", property: "transform-origin", value: null },
];
const result = sdkShadowDispatch(session, "hf-box", ops);
// ... but the SDK still has it → genuine value_mismatch, not suppressed.
// The SDK now removes the hyphenated property, so the shadow read-back agrees.
expect(result.dispatched).toBe(true);
expect(result.mismatches).toHaveLength(1);
expect(result.mismatches[0]).toMatchObject({
kind: "value_mismatch",
property: "transform-origin",
expected: null,
actual: "center center",
});
expect(result.mismatches).toHaveLength(0);
});
it("applies attribute op and reads back via session.getElement", async () => {