From dfe63af3ae45a68ccc5de4d9648507f1ab9cf708 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 8 Jul 2026 09:52:48 -0700 Subject: [PATCH] fix(core,cli): parent-relative figma child geometry; groups stop rejecting subcommand flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found running the brand-loop guide end-to-end against the Simple Design System: - nodeToHtml subtracted the ROOT origin from every node's absolute bounds, but CSS absolute positioning resolves against the nearest positioned ancestor — every nesting level re-added its ancestors' offsets, drifting nested content down-right and pushing deep children off-frame (hero buttons invisible, pricing grid collapsed to one card). Children now subtract their PARENT's box; regression test with a two-level tree. - trackCommandFailures asserted unknown flags against the command group's own (flagless) arg table even when the group was delegating to a subcommand, so `figma component --name x` imported and THEN threw "Unknown flag: --name". The assertion is now skipped when the first positional names a subcommand; leaf and non-delegating behavior is unchanged and covered by tests. Co-Authored-By: Claude Fable 5 --- .../utils/command-failure-tracking.test.ts | 54 +++++++++++++++++++ .../cli/src/utils/command-failure-tracking.ts | 18 ++++++- packages/core/src/figma/nodeToHtml.test.ts | 32 +++++++++++ packages/core/src/figma/nodeToHtml.ts | 24 ++++++--- 4 files changed, 120 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/utils/command-failure-tracking.test.ts b/packages/cli/src/utils/command-failure-tracking.test.ts index 1798d0ef2..0f7475af1 100644 --- a/packages/cli/src/utils/command-failure-tracking.test.ts +++ b/packages/cli/src/utils/command-failure-tracking.test.ts @@ -39,6 +39,60 @@ describe("trackCommandFailures", () => { expect(onFailure).not.toHaveBeenCalled(); }); + it("rejects an unknown flag on a leaf command", async () => { + const wrapped = trackCommandFailures( + () => + Promise.resolve({ + meta: { name: "leaf" }, + args: { out: { type: "string" } }, + run: () => Promise.resolve(), + } as CommandDef), + vi.fn(), + ); + const cmd = await wrapped(); + await expect( + (cmd.run as (ctx: unknown) => Promise)({ rawArgs: ["--bogus", "x"] }), + ).rejects.toThrow(/--bogus/); + }); + + it("skips the unknown-flag check when a command group delegates to a subcommand", async () => { + // `figma component --name x`: --name belongs to the subcommand's + // table; the group (subCommands + fallback-help run) must not reject it. + const run = vi.fn(() => Promise.resolve()); + const wrapped = trackCommandFailures( + () => + Promise.resolve({ + meta: { name: "figma" }, + subCommands: { component: () => Promise.resolve({ meta: { name: "component" } }) }, + run, + } as unknown as CommandDef), + vi.fn(), + ); + const cmd = await wrapped(); + await expect( + (cmd.run as (ctx: unknown) => Promise)({ + rawArgs: ["component", "KEY:1-2", "--name", "hero"], + }), + ).resolves.toBeUndefined(); + expect(run).toHaveBeenCalled(); + }); + + it("still rejects an unknown flag when the group is NOT delegating", async () => { + const wrapped = trackCommandFailures( + () => + Promise.resolve({ + meta: { name: "figma" }, + subCommands: { component: () => Promise.resolve({ meta: { name: "component" } }) }, + run: () => Promise.resolve(), + } as unknown as CommandDef), + vi.fn(), + ); + const cmd = await wrapped(); + await expect( + (cmd.run as (ctx: unknown) => Promise)({ rawArgs: ["--bogus"] }), + ).rejects.toThrow(/--bogus/); + }); + it("passes through a command with no run() untouched", async () => { const onFailure = vi.fn(); const parent: CommandDef = { meta: { name: "parent" } }; diff --git a/packages/cli/src/utils/command-failure-tracking.ts b/packages/cli/src/utils/command-failure-tracking.ts index d5bc1d435..efe65ef5e 100644 --- a/packages/cli/src/utils/command-failure-tracking.ts +++ b/packages/cli/src/utils/command-failure-tracking.ts @@ -54,7 +54,23 @@ function wrapCommand( // Reject unknown flags before the command body: citty silently ignores // them otherwise, dropping the value (e.g. `render --out x` fell back to // the default output path). Inside the try so the rejection is reported. - assertKnownFlags(cmd, ctx?.rawArgs ?? []); + // + // A command group (subCommands + fallback-help run) delegating to a + // subcommand must NOT assert here: the flags belong to the subcommand's + // table, not the group's, and would be falsely rejected (e.g. + // `figma component --name x`). The wrapped subcommand loaders + // below assert the leaf's own table, so typo protection is preserved. + // Heuristic caveat: "first non-dash token names a subcommand" is sound + // only while command groups declare no flags of their own — if a group + // grows a flag whose value could match a subcommand name, replace this + // with a real argv parse. + const rawArgs = ctx?.rawArgs ?? []; + const firstPositional = rawArgs.find((tok) => tok && !tok.startsWith("-")); + const delegatesToSub = + cmd.subCommands != null && + firstPositional != null && + Object.prototype.hasOwnProperty.call(cmd.subCommands, firstPositional); + if (!delegatesToSub) assertKnownFlags(cmd, rawArgs); return await run(ctx); } catch (err) { try { diff --git a/packages/core/src/figma/nodeToHtml.test.ts b/packages/core/src/figma/nodeToHtml.test.ts index 8c4b1f9b2..7f70a61dc 100644 --- a/packages/core/src/figma/nodeToHtml.test.ts +++ b/packages/core/src/figma/nodeToHtml.test.ts @@ -52,6 +52,38 @@ describe("nodeToHtml", () => { expect(out.html).toContain("background-color: #0066FF"); }); + it("positions nested children relative to their PARENT, not the root", () => { + const out = nodeToHtml( + frame([ + { + id: "1:2", + name: "Group", + type: "FRAME", + absoluteBoundingBox: BOX(600, 400, 300, 200), + children: [ + { + id: "1:3", + name: "Inner", + type: "RECTANGLE", + absoluteBoundingBox: BOX(620, 440, 100, 40), + fills: [SOLID_BLUE], + }, + ], + }, + ]), + { resolved: [], unresolved: [] }, + ); + // Group at canvas (600,400) inside root (100,200) → left 500, top 200. + expect(out.html).toContain("left: 500px"); + expect(out.html).toContain("top: 200px"); + // Inner at canvas (620,440) inside Group (600,400) → left 20, top 40 — + // NOT root-relative 520/240, which double-offsets when CSS resolves + // absolute position against the positioned parent. + expect(out.html).toContain("left: 20px"); + expect(out.html).toContain("top: 40px"); + expect(out.html).not.toContain("left: 520px"); + }); + it("emits var() with literal fallback for resolved bindings", () => { const out = nodeToHtml( frame([ diff --git a/packages/core/src/figma/nodeToHtml.ts b/packages/core/src/figma/nodeToHtml.ts index 102567833..6ce0c81d8 100644 --- a/packages/core/src/figma/nodeToHtml.ts +++ b/packages/core/src/figma/nodeToHtml.ts @@ -186,7 +186,7 @@ function unresolvedAttr(node: FigmaNodeDocument, ctx: RenderContext): string { return ` data-figma-unresolved="${escapeHtml(props.join(" "))}"`; } -function geometryCss(node: FigmaNodeDocument, ctx: RenderContext, isRoot: boolean): string[] { +function geometryCss(node: FigmaNodeDocument, parentBox: Box, isRoot: boolean): string[] { const box = boxOf(node); const styles: string[] = []; if (!box) return styles; @@ -197,10 +197,14 @@ function geometryCss(node: FigmaNodeDocument, ctx: RenderContext, isRoot: boolea `height: ${round(box.height)}px`, ); } else { + // CSS absolute positioning is relative to the nearest positioned + // ancestor — the PARENT's box, not the root origin. Subtracting the root + // for every depth double-offsets nested children (each level re-adds its + // ancestors' offsets), drifting content down-right and off-frame. styles.push( "position: absolute", - `left: ${round(box.x - ctx.origin.x)}px`, - `top: ${round(box.y - ctx.origin.y)}px`, + `left: ${round(box.x - parentBox.x)}px`, + `top: ${round(box.y - parentBox.y)}px`, `width: ${round(box.width)}px`, `height: ${round(box.height)}px`, ); @@ -243,10 +247,15 @@ function decorationCss(node: FigmaNodeDocument, ctx: RenderContext): string[] { // instead of a RangeError; real figma frames are nowhere near this deep. const MAX_DEPTH = 500; -function renderChildren(node: FigmaNodeDocument, ctx: RenderContext, depth: number): string { +function renderChildren( + node: FigmaNodeDocument, + ctx: RenderContext, + depth: number, + parentBox: Box, +): string { const childHtml: string[] = []; for (const child of childDocuments(node)) { - const rendered = renderNodeHtml(child, ctx, false, depth + 1); + const rendered = renderNodeHtml(child, ctx, false, depth + 1, parentBox); if (rendered.length > 0) childHtml.push(rendered); } return childHtml.length > 0 ? `\n${childHtml.join("\n")}\n` : ""; @@ -257,11 +266,12 @@ function renderNodeHtml( ctx: RenderContext, isRoot: boolean, depth = 0, + parentBox: Box = ctx.origin, ): string { if (node.visible === false || depth > MAX_DEPTH) return ""; const slug = uniqueSlug(ctx, node.name); const style = escapeHtml( - [...geometryCss(node, ctx, isRoot), ...decorationCss(node, ctx)].join("; "), + [...geometryCss(node, parentBox, isRoot), ...decorationCss(node, ctx)].join("; "), ); // data-hf-snippet marks the file as a mountable fragment, not a standalone // composition — the project linter skips composition-root rules for it. @@ -278,7 +288,7 @@ function renderNodeHtml( return `
${text}
`; } - return `
${renderChildren(node, ctx, depth)}
`; + return `
${renderChildren(node, ctx, depth, boxOf(node) ?? parentBox)}
`; } export interface NodeToHtmlOptions {