feat(core,cli): figma component import with binding-aware node-to-html mapper (M3)

[vanceingalls]

What

M3: Phase-3 component import — node tree → editable HTML, with the §7.1 binding pass wired in.

• resolveBindings.ts — scans the complete tree before any CSS is emitted: boundVariables (single alias objects and arrays — the consumer-side shape was later confirmed live) plus style-id slots (styles: { fill: … } → style:fill sites), children recursed. Partitions against the binding index exact-ID-only (id, alias-chain membership, or stable key) into resolved / unresolved. A missed link bakes a correct literal; a wrong link would silently change color at the next brand refresh — so no value/name matching, ever.
• nodeToHtml.ts — hybrid fidelity routing per spec §7: exact absolute geometry inside a fixed-size root (no reflow, no drift); CSS where CSS is faithful (solid/linear-gradient fills, radius, opacity, drop shadow, blur, text styles); resolved bindings emit var(--slug, #literal) (works even before the host defines the var); unresolved bake the literal + data-figma-unresolved; visible:false nodes and fills skipped (Figma's own semantics — the MCP exporter gets this wrong); vectors/boolean-ops route to a rasterize list; stable slug ids + data-figma-id on every element so Phase-4 motion can target them.
• hyperframes figma component <ref> — tree → bindings → HTML; rasterize fallback runs Phase-1 asset export per flagged node and backfills the src; packaged as a registry-item.json; unresolved bindings produce actionable guidance (run tokens on the source/library file, re-import to link).

Tests

resolveBindings partition cases (indexed, alias-chain, unknown→unresolved, style sites, empty tree), nodeToHtml rendering matrix (geometry, var()-fallback, unresolved flagging, text escaping+font styles, rasterize routing, invisible-node/fill skipping, gradients+shadows), CLI end-to-end with fake client (var() emission, literal+flag path, rasterize+registry packaging).

---

Stack (5/6): #1868 → #1869 → #1870 → #1871 → this PR → #1873

🤖 Generated with Claude Code

[vanceingalls]

• feat(skills): reroute /figma by capability - REST/CLI for phases 1-3, MCP for 4-5 (M4) #1873
• feat(core,cli): figma component import with binding-aware node-to-html mapper (M3) #1872 👈 (View in Graphite)
• feat(core,cli): figma tokens import with alias-aware binding records (M2) #1871
• feat(core,cli): figma REST client, asset import command, binding index (M0+M1) #1870
• feat(core): figma motion → GSAP translator + /figma skill v1 #1869
• feat(core): figma module foundations — types, parseFigmaRef, freeze, manifest, asset snippet #1868
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[james-russo-rames-d-jusso]

Reviewed at 5473a249af (stack 5/6, chain #1868→…→#1873).
Body carries 🤖 footer + AI-trailer per HF convention; noted.

Summary — Adds the Phase-3 mapper: figma node tree → editable HTML with a binding pass that emits var(--slug, #literal) for resolved bindings and data-figma-unresolved for missed ones. Well-shaped tests (partition matrix + rendering matrix + CLI e2e). One HTML-injection surface via unescaped font-family, one string-replace fragility in the rasterize backfill.

Concerns

🔴 packages/core/src/figma/nodeToHtml.ts:154 — HTML injection via unescaped fontFamily in the style attribute.

if (typeof s.fontFamily === "string") styles.push(`font-family: '${s.fontFamily}'`);

The CSS chunk goes into style="${style}" at lines 251/256/259 with no HTML escape on style. Figma text nodes carry style.fontFamily as arbitrary strings; a Figma file authored (or edited) with fontFamily: Malicious"; onerror="alert(1)"renders as<div ... style="font-family: 'Malicious"; onerror="alert(1)"'">and the parser breaks out of thestyleattribute into a newonerrorattribute on the same tag. Every other CSS chunk is machine-derived (numbers, hex colors fromfigmaColorToCss), but fontFamilyandcharactersare the two content-controlled strings that reach emission.charactersis escaped at line 255;fontFamilyis not. Fix: escape the fontFamily viaescapeHtml(or strip to a font-name allowlist[A-Za-z0-9 _-]`) before quoting. Same treatment worth adding to any future style path that flows user-controlled strings (letter-spacing suffix, custom easing names, etc.).

Threat model: even without malicious authoring, a designer with a smart-quote paste in the font name breaks the style attribute silently. This is not purely a "hostile Figma file" concern.

🟠 packages/cli/src/commands/figma/component.ts:62-65 — .replace on raw nodeId will silently no-op if nodeToHtml escaped the id.

html = html.replace(
  `data-figma-rasterize="${req.nodeId}" `,
  `data-figma-rasterize="${req.nodeId}" src="${srcRel}" `,
);

Two failure modes:

1. nodeToHtml.ts:251 emits data-figma-rasterize="${escapeHtml(node.id)}", but the replace searches the raw req.nodeId. Figma IDs are <num>:<num> in the common case so this works today — but the escape-vs-raw drift is the kind of thing that breaks the day someone lets a synthetic-id feature merge.
2. String.prototype.replace(string, string) only replaces the first occurrence. If the same figma node appears twice in the rasterize list (nested duplicate reference, or the same shared component instantiated twice in a frame), the second placeholder never gets its src. Since RasterizeRequest includes a per-node slug that IS unique-per-render, keying the replace on slug instead of nodeId (or splicing on the element directly) removes both risks.

Also srcRel isn't HTML-escaped before it lands in the attribute value; on odd path shapes (Windows quotes) it breaks. Nit-adjacent, but pair it with the fix above.

🟠 packages/cli/src/commands/figma.ts (this PR, unchanged file) — meta description says "assets, tokens, and components (REST)" but subcommand map still only wires asset and tokens.

The PR body says "packaged as a registry-item.json" and the CLI entry command is documented in the description, but hyperframes figma component <ref> doesn't resolve until #1873 lands the routing wire-up. Per-PR-isolation, this PR ships the mapper + tests + a component.ts command file that no dispatcher can reach. That's fine as a Graphite stack shape, but a reviewer looking at #1872 in isolation could reasonably ask whether the wiring omission is intentional. Sibling PR #1873 makes it a non-issue on merge — flagging so intent is on the record.

Nits

🟡 packages/core/src/figma/nodeToHtml.ts:112-114 — gradientCss hardcodes 180deg with a "ponytail" comment. Not a blocker (spec §7.1 probe list acknowledges it), but any imported gradient will render with the wrong angle relative to Figma's handle-space until the probe lands. Worth a data-figma-gradient-angle="approx" marker so a follow-up tool can spot which nodes need re-emission when the exact math ships.

🟡 packages/core/src/figma/resolveBindings.ts:73-80 — findInIndex is O(N·M) over sites × index entries. For a 10k-node file × a 500-entry binding index that's 5M comparisons on every import. Fine at current scale; if binding indexes grow (multiple libraries × modes × styles), replacing the linear scan with a Map keyed by figmaId + a second map for alias-chain membership will be worth it.

🟡 packages/core/src/figma/nodeToHtml.ts + resolveBindings.ts — recursion depth uncapped. renderNodeHtml and collectSites recurse without a depth limit; V8 default is ~10k frames so realistically fine, but a designer with a runaway auto-generated tree would hit RangeError instead of a graceful skip. One-line depth guard on both paths is cheap insurance.

🟡 packages/core/src/figma/nodeToHtml.ts:184 — resolved.find per node × per property is another linear scan. Same class of concern as the resolveBindings one; a Map<${nodeId}:${property}, compositionVariableId> shaped once at the top of nodeToHtml amortizes it.

Questions

↩️ Collision handling when two Figma nodes bind to the same composition variable, or two nodes slugify to the same base name in the SAME resolved binding. uniqueSlug handles the id-collision path with -2/-3 suffixes. But when two nodes share a figmaId binding target and both get resolved, do we want independent var(--slug, …) sites (current behavior — fine) or is there a downstream expectation that the composition variable is emitted once and referenced from both? Not blocking, but worth confirming against the runtime getVariables consumer contract.

↩️ Empty-string / all-non-alphanum node names. slugify("!!!") → "" → fallback "node". Two such nodes get node + node-2. Confirmed via the usedSlugs set. That's fine — flagging so the fallback isn't a surprise in output.

↩️ Any expectation that unresolved sites reappear across re-imports? After running tokens and re-importing the component, the current CLI overwrites the html file (writeFileSync, no dirty-check). If a user hand-edited the component between imports, their edits get clobbered. Not this PR's problem — but worth capturing as a follow-up.

What I didn't verify

• The gradientHandlePositions math referenced in spec §7.1 (probe-flagged; the ponytail 180deg is deliberate).
• Runtime rendering of the emitted HTML against the actual HyperFrames runtime — reviewed the emission shape only, not the round-trip.
• Behavior against a real 10k-node Figma file (performance nits above are static-analysis, not measured).
• Whether sanitizeSvg (invoked via Phase-1 asset export during rasterize) covers the SVG shapes that Figma's /v1/images endpoint produces for BOOLEAN_OPERATION in practice — read the sanitizer separately at a prior stack member.

— Rames D Jusso

[miga-heygen]

Review — M3 component import (binding-aware node-to-HTML mapper)

Solid Phase-3 delivery. The three-module split (resolveBindings → nodeToHtml → component CLI) is clean, the binding resolution pass is correctly positioned before CSS emission, and the exact-ID-only matching rule is well-enforced. The tolerance for both single-alias and array-of-alias boundVariables shapes is a smart defensive move given the probe-flagged consumer-side shape. Tests cover the critical paths well.

One bug, a few concerns worth discussion.

---

Bug

1. TEXT node fill bindings are silently dropped

In decorationCss() (nodeToHtml.ts), TEXT nodes get their color via fillCss(node) — the raw literal path — instead of backgroundValue(node, ctx) which does the binding-resolution var() wrapping. The bg variable IS computed through backgroundValue(), but it sits in the else branch and is never used for TEXT:

const bg = backgroundValue(node, ctx);
if (node.type === "TEXT") {
    const color = fillCss(node);           // ← literal only, bindings ignored
    if (color !== null) styles.push(`color: ${color}`);
} else if (bg !== null) {
    styles.push(`background: ${bg}`);      // ← binding-aware path
}

If a TEXT node's fill color is bound to a design token that's in the resolved index, the token link is silently lost — the literal gets baked, and a brand refresh via CSS variables won't propagate. The fix: route TEXT color through a binding-aware path (e.g. backgroundValue(node, ctx) or a colorValue() variant that checks resolved bindings for fills). There's no test for TEXT + resolved binding, which is how this slipped through.

---

Concerns (non-blocking, worth discussing)

2. ELLIPSE nodes render as rectangles

ELLIPSE isn't in RASTERIZE_TYPES and isn't TEXT, so it falls through to the generic <div> path. Figma doesn't typically set cornerRadius on ELLIPSE — the node is inherently elliptical. CSS can handle this perfectly with border-radius: 50%, but the current code won't emit it. Either add ELLIPSE to the rasterize list or add a type-check that emits border-radius: 50%.

3. Missing overflow: hidden for clipped frames

Figma frames have clipsContent: boolean. When true, children outside bounds should be clipped. The mapper doesn't check this property or emit overflow: hidden, so children can bleed past their parent frame visually. If this is deferred to a future probe pass, a comment would help.

4. Component slug collision across separate imports

runComponentImport derives the output directory from slugify(tree.name). Two components with names that slugify identically (e.g. "Hero Card" vs "Hero—Card") would silently overwrite each other's files. uniqueSlug() in nodeToHtml.ts handles intra-component slug dedup, but nothing guards inter-import collisions. A check-exists-and-warn or auto-suffix would prevent silent data loss.

5. extraOpacity parameter on figmaColorToCss is unused in this PR

The extraOpacity param in figmaColorToCss(value, extraOpacity = 1) is never called with a non-default value anywhere in this PR. It defaults harmlessly to 1, so no functional issue — just noting it's forward-declared dead code. If it's planned for fill-opacity compositing in a later phase, all good.

---

Nits (non-blocking)

6. Linear scan lookups in backgroundValue and unresolvedAttr

Both do .find() / .filter() over the full resolved/unresolved arrays for every node — O(nodes × bindings). A Map<nodeId, sites[]> built once in nodeToHtml() would make each lookup O(1). Not a practical bottleneck for typical Figma frames, but a free win if large-file perf matters later.

7. Same pattern in findInIndex (resolveBindings.ts)

Linear scan of the entire binding index per binding site, with nested aliasChain?.includes(). A Map keyed by figmaId + secondary entries for alias chain members would be O(1) per lookup. Same caveat — not a practical problem at current scale.

---

net: ~8 lines possible (Map-based lookup refactor for #6/#7 would add a few lines but drop per-node scan code; TEXT binding fix in #1 is ~3 lines changed)

— Miga

🤖 Generated with Claude Code

[vanceingalls]

Review feedback addressed (pushed in the absorbed update):

Fixed

• fontFamily injection (Rames 🔴): quote/escape chars are stripped from fontFamily before it reaches CSS, AND the whole style attribute is HTML-escaped at emission — breakout into a new attribute is dead on both layers. Test asserts a hostile Mal"; onerror="alert(1) font name can't produce an onerror attribute. (Smart-quote paste in a font name also survives now.)
• TEXT fill bindings silently dropped (miga bug Initial repo setup #1): TEXT color now routes through the binding-aware backgroundValue path — a token-bound text fill emits color: var(--slug, literal) and keeps its brand link. Test with a resolved TEXT binding added (the missing test you called out).
• Rasterize backfill fragility (Rames 🟠): the replace key is now the EMITTED (escaped) node id, uses replaceAll (same node appearing twice both get their src), and srcRel is attribute-escaped.
• ELLIPSE renders as rectangle (miga initial code #2): ELLIPSE nodes emit border-radius: 50%. Test added.
• clipsContent (miga feat(core): add compiler entry point and runtime composition fixes #3): emits overflow: hidden. Test added.
• Component dir collision (miga feat(studio): consolidate into single OSS-ready NLE editor #4): existing dir now warns before overwrite with a rename hint.
• Recursion depth (Rames 🟡): 500-frame depth cap, degrade-to-skip.

Deliberately deferred

• Gradient angle (Rames 🟡): stays probe-flagged per spec §7.1; the ponytail comment marks it.
• Linear-scan → Map refactors (miga feat(core): add compiler entry point and runtime composition fixes #6/feat(studio): consolidate into single OSS-ready NLE editor #7, Rames 🟡): agreed on the shape, deferred until file sizes make it measurable.
• Re-import overwriting hand-edits (Rames Q3): real follow-up, captured in spec §13 alongside the refresh command.

Answers

• Two nodes bound to the same variable → independent var(--slug, …) sites is intended; the composition variable is defined once in data-composition-variables, referenced N times.
• slugify("!!!") → node/node-2 fallback: confirmed intended.
• CLI wiring for component landed in feat(skills): reroute /figma by capability - REST/CLI for phases 1-3, MCP for 4-5 (M4) #1873 as you noted.

🤖 Generated with Claude Code

[miga-heygen]

Re-review — M3 component import (post-feedback revision)

Re-reviewed at 2238d2f0 (force-push incorporating review feedback). Previous review requested changes on the same commit scope.

All five concerns from the previous round have been addressed. The author force-pushed a revised commit that incorporates fixes and added a dedicated "nodeToHtml review fixes" test suite covering each change. Clean work.

Previous concerns — status

1. TEXT node fill bindings silently dropped — FIXED. decorationCss() now routes TEXT color through backgroundValue(node, ctx) — the same binding-aware path used for background fills. The bg variable is computed once and used in both the TEXT branch (color: ${bg}) and the non-TEXT branch (background: ${bg}). Comment at the top of decorationCss explicitly documents the intent. Test added: "routes TEXT color through the binding-aware path (var() when resolved)."

2. ELLIPSE nodes render as rectangles — FIXED. decorationCss() now checks node.type === "ELLIPSE" and emits border-radius: 50%. Falls through to the explicit cornerRadius check only for non-ELLIPSE types. Test added: "renders ELLIPSE with border-radius 50%."

3. Missing overflow: hidden for clipped frames — FIXED. decorationCss() now checks node.clipsContent === true and emits overflow: hidden. Test added: "emits overflow hidden for clipsContent frames."

4. Component slug collision — ACKNOWLEDGED. runComponentImport now warns via console.warn when the target directory already exists, noting the overwrite and suggesting a rename. Not a hard guard, but sufficient for the CLI context — the user sees the warning and can act.

5. extraOpacity parameter unused — UNCHANGED. Still defaults to 1, never called with a non-default value. Forward-declared for fill-opacity compositing — harmless, acknowledged.

Also addressed (from Rames's review)

• HTML injection via fontFamily: now strips '"\; characters before quoting. The full style string is also HTML-escaped at emission (defense in depth). Test added: "neutralizes a hostile fontFamily instead of breaking out of the style attribute."
• String.replace fragility in rasterize backfill: now uses escapeAttr() on the nodeId to match the escaped emission, replaceAll instead of replace, and escapes srcRel. Both the escape-drift and single-occurrence risks eliminated.
• Recursion depth uncapped: MAX_DEPTH = 500 guard added to renderNodeHtml. collectSites in resolveBindings still recurses without a cap, but the practical risk is low (same tree, and 500-deep Figma frames don't exist in practice).

New findings

None. The revised code is clean. The linear-scan nits (O(N·M) in backgroundValue/findInIndex) from both reviews remain as-is — acknowledged as non-blocking at current scale, and correctly deferred.

CI

All required checks pass. Graphite mergeability pending (standard for stacked PRs). Mintlify skipped (no docs changes).

— Miga

🤖 Generated with Claude Code

[miguel-heygen]

LGTM. Stamping after Miga R2 verified the prior Figma-stack blockers are addressed; live checks are green, with only Graphite stack mergeability pending where applicable.

[james-russo-rames-d-jusso]

R2 verification — reviewed at 2238d2f0 (post-force-push). 🟢

Both R1 findings fixed and tested. Layering onto Miga's R2 (4627989494 at 20:49:52Z), same commit — she reached the same verdict via a wider concern set (five previous, plus mine).

R1 🔴 HTML injection via unescaped fontFamily — resolved

Verified at nodeToHtml.ts:151-164:

if (typeof s.fontFamily === "string")
  styles.push(`font-family: '${s.fontFamily.replace(/['"\\;]/g, "")}'`);

Then defense-in-depth at emission — the full style string is HTML-escaped before landing in style="..." (line 264: const style = escapeHtml(...)), so any residual < / > get neutralized too.

Mechanism analysis:

• Strips ', ", \, ; from the content-controlled fontFamily value → can't close the single-quoted CSS font-family value or terminate the CSS declaration.
• HTML-escapes &, <, >, " at the whole-style level → can't break out of the style="..." attribute even if a > slipped through the CSS-context filter.
• Two independent layers, either sufficient alone. Correct rubric.

Test at nodeToHtml.test.ts:245-257 uses the classic breakout payload Mal"; onerror="alert(1) and asserts neither onerror=" nor style="[^"]*" onerror appear in output. Solid — that's the actual attack shape.

Sibling asymmetry check: grepped packages/core/src/figma/ + packages/cli/src/commands/figma/ for other fontFamily / font-family write sites — only one (line 158). No parallel path could regress.

Composition with #1868's REST fetch shape: FigmaNodeDocument is Record<string, unknown>-shaped, s.fontFamily is unknown until the typeof === "string" guard runs, so a non-string upstream value degrades to no-emit rather than throwing. Clean.

R1 :large_orange_circle: F2 .replace first-match on data-URI — resolved

Verified at component.ts:66-81:

const emittedId = escapeAttr(req.nodeId);
html = html.replaceAll(
  `data-figma-rasterize="${emittedId}" `,
  `data-figma-rasterize="${emittedId}" src="${escapeAttr(srcRel)}" `,
);

Three fixes stacked: (a) .replace → .replaceAll covers the same node appearing twice in the tree, (b) escapeAttr(nodeId) on the search key matches the HTML-escaped form emitted by nodeToHtml.ts:271, so the search key never drifts from the emitted key, (c) srcRel itself is escaped in the replacement so a path containing & / < / " can't inject either. Comment at line 65-67 documents the invariant.

Also verified

• MAX_DEPTH=500 guard added at nodeToHtml.ts:243-262 — renderNodeHtml returns "" past the cap, so a runaway auto-generated tree degrades to a skip instead of stack-overflowing.

Verdict

✅ LGTM. Both my R1 findings addressed with the right mechanism + real tests. Stack composes cleanly with #1868/#1871 upstream.

CI all green (perf, preview-regression, regression shards 1-8, player-perf); Graphite mergeability pending (standard for stacked PRs); Mintlify skipped (non-docs).

— Rames D Jusso

The base branch was changed.

[github-actions]

Fallow audit report

Found 4 findings.

Dead code (1)

Severity	Rule	Location	Description
major	fallow/unused-export	packages/cli/src/commands/figma/component.ts:116	Export 'default' is never imported by other modules

Duplication (2)

Severity	Rule	Location	Description
minor	fallow/code-duplication	packages/cli/src/commands/figma/download.ts:5	Code clone group 1 (10 lines, 2 instances)
minor	fallow/code-duplication	packages/core/src/figma/freeze.ts:53	Code clone group 1 (10 lines, 2 instances)

Health (1)

Severity	Rule	Location	Description
minor	fallow/high-crap-score	packages/core/src/figma/nodeToHtml.ts:220	'decorationCss' has CRAP score 31.6 (threshold: 30.0, cyclomatic 10)

Generated by fallow.