diff --git a/packages/mcp/src/tools/binding-schema.ts b/packages/mcp/src/tools/binding-schema.ts new file mode 100644 index 0000000..daa653e --- /dev/null +++ b/packages/mcp/src/tools/binding-schema.ts @@ -0,0 +1,20 @@ +import { z } from 'zod'; + +/** + * Variable bindings on the object that owns them — `{ field: variableId }` — as get_node / + * get_styles report them. Shared by the paint / effect / grid input schemas so a value read out of + * Figma writes straight back with its bindings intact instead of being flattened to the literal + * beside it (issue #164). + * + * These arrays replace rather than patch: a paint or effect written back WITHOUT `boundVariables` + * clears whatever it was bound to. (set_text_range is the exception — being a patch, it takes an + * explicit null to unbind.) + */ +export const boundVariablesSchema = z + .record(z.string(), z.string()) + .describe( + 'Variable bindings for this object as { field: variableId }, e.g. { "color": "VariableID:5:12" } ' + + '— round-trips what get_node / get_styles report. The bound field then tracks the variable ' + + 'instead of the literal next to it. Omit to leave the value unbound: writing without it ' + + 'CLEARS any binding the object had.', + ); diff --git a/packages/mcp/src/tools/create-grid-style.ts b/packages/mcp/src/tools/create-grid-style.ts index 288b127..23898c1 100644 --- a/packages/mcp/src/tools/create-grid-style.ts +++ b/packages/mcp/src/tools/create-grid-style.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; +import { gridItemSchema } from './grid-schema.js'; import type { ToolSpec } from './spec.js'; export const CREATE_GRID_STYLE_TOOL_NAME = 'create_grid_style'; @@ -12,17 +13,7 @@ export const createGridStyleTool: ToolSpec = { 'to frames with apply_style_to_node. Returns { ok, styleId, name }.', inputSchema: z.object({ name: z.string().describe('Style name, e.g. "Layout/8pt"'), - grids: z.array( - z.object({ - pattern: z.enum(['GRID', 'ROWS', 'COLUMNS']), - visible: z.boolean(), - sectionSize: z.number().optional(), - count: z.number().optional(), - gutterSize: z.number().optional(), - alignment: z.enum(['MIN', 'MAX', 'CENTER', 'STRETCH']).optional(), - offset: z.number().optional(), - }), - ), + grids: z.array(gridItemSchema), description: z.string().optional(), }), kind: 'write', diff --git a/packages/mcp/src/tools/effect-schema.ts b/packages/mcp/src/tools/effect-schema.ts index affa893..459ae29 100644 --- a/packages/mcp/src/tools/effect-schema.ts +++ b/packages/mcp/src/tools/effect-schema.ts @@ -1,5 +1,7 @@ import { z } from 'zod'; +import { boundVariablesSchema } from './binding-schema.js'; + // Shared Zod effect schema, reused by set_effects / create_effect_style so the shadow + blur shape // can't drift between them (they previously copy-pasted the same inline JSON shape). Loose so an // effect read back from get_node round-trips into a write. The plugin's toFigmaEffect enforces that @@ -19,5 +21,10 @@ export const effectItemSchema = z .describe('Shadow offset in px. Required for shadows.') .optional(), spread: z.number().optional(), + boundVariables: boundVariablesSchema + .describe( + 'Bindable fields: color (COLOR variable) and radius / spread / offsetX / offsetY (FLOAT).', + ) + .optional(), }) .loose(); diff --git a/packages/mcp/src/tools/grid-schema.ts b/packages/mcp/src/tools/grid-schema.ts new file mode 100644 index 0000000..e3475c0 --- /dev/null +++ b/packages/mcp/src/tools/grid-schema.ts @@ -0,0 +1,28 @@ +import { z } from 'zod'; + +import { boundVariablesSchema } from './binding-schema.js'; + +// Shared Zod layout-grid schema, reused by set_layout_grids / create_grid_style so the shape can't +// drift between them (they previously carried the same inline object twice, and the copies had +// already diverged in their descriptions). Loose, like the paint and effect schemas, so a grid read +// back from get_node round-trips into a write. The plugin's toFigmaLayoutGrid enforces the rest +// (GRID needs sectionSize; ROWS/COLUMNS need count + gutterSize; CENTER rejects offset). + +/** One layout grid: GRID (uniform squares) or ROWS / COLUMNS (count + gutter + alignment). */ +export const gridItemSchema = z + .object({ + pattern: z.enum(['GRID', 'ROWS', 'COLUMNS']), + visible: z.boolean(), + sectionSize: z + .number() + .optional() + .describe('Cell size for GRID; section size for ROWS/COLUMNS (ignored when STRETCH)'), + count: z.number().optional().describe('Number of columns/rows (ROWS/COLUMNS)'), + gutterSize: z.number().optional().describe('Gap between columns/rows (ROWS/COLUMNS)'), + alignment: z.enum(['MIN', 'MAX', 'CENTER', 'STRETCH']).optional(), + offset: z.number().optional().describe('Page margin from the frame edge (ignored when CENTER)'), + boundVariables: boundVariablesSchema + .describe('Bindable fields: sectionSize / count / offset / gutterSize (FLOAT variables).') + .optional(), + }) + .loose(); diff --git a/packages/mcp/src/tools/paint-schema.ts b/packages/mcp/src/tools/paint-schema.ts index 17aad89..df17576 100644 --- a/packages/mcp/src/tools/paint-schema.ts +++ b/packages/mcp/src/tools/paint-schema.ts @@ -1,5 +1,7 @@ import { z } from 'zod'; +import { boundVariablesSchema } from './binding-schema.js'; + // Shared Zod paint schema, reused by set_fills / set_strokes / create_paint_style / // update_paint_style so the SOLID + gradient shape can't drift between them. Loose (only `type` is // required and unknown keys pass through) so a paint read back from get_node round-trips into a @@ -22,7 +24,14 @@ export const paintItemSchema = z ]), color: rgb.describe('SOLID color (r/g/b in 0–1)').optional(), gradientStops: z - .array(z.object({ position: z.number(), color: rgba })) + .array( + z.object({ + position: z.number(), + color: rgba, + // A stop binds its colour independently of its siblings, so the binding rides here. + boundVariables: boundVariablesSchema.optional(), + }), + ) .describe('Gradient stops (position 0–1 + RGBA color); required for gradient types') .optional(), gradientTransform: z @@ -31,5 +40,8 @@ export const paintItemSchema = z .optional(), opacity: z.number().optional(), visible: z.boolean().optional(), + boundVariables: boundVariablesSchema + .describe('SOLID only: { "color": variableId } binds the paint colour to a COLOR variable.') + .optional(), }) .loose(); diff --git a/packages/mcp/src/tools/set-fills.ts b/packages/mcp/src/tools/set-fills.ts index 5f818e0..f739e85 100644 --- a/packages/mcp/src/tools/set-fills.ts +++ b/packages/mcp/src/tools/set-fills.ts @@ -10,7 +10,9 @@ export const setFillsTool: ToolSpec = { description: "Set a node's fills. SOLID: { type:'SOLID', color:{r,g,b} } (0–1). Gradient: " + "{ type:'GRADIENT_LINEAR'|…, gradientStops:[{position,color:{r,g,b,a}}], gradientTransform } " + - '(round-trips get_node output). Returns { ok, nodeId }.', + '(round-trips get_node output). A SOLID paint may carry boundVariables ({ color: variableId }) ' + + 'and a gradient stop its own — the paint then tracks that variable instead of the literal. ' + + 'Returns { ok, nodeId }.', inputSchema: z.object({ nodeId: z.string().describe('Figma node id to repaint'), fills: z.array(paintItemSchema).describe('Paints to apply'), diff --git a/packages/mcp/src/tools/set-layout-grids.ts b/packages/mcp/src/tools/set-layout-grids.ts index 94ccf2f..0a77764 100644 --- a/packages/mcp/src/tools/set-layout-grids.ts +++ b/packages/mcp/src/tools/set-layout-grids.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; +import { gridItemSchema } from './grid-schema.js'; import type { ToolSpec } from './spec.js'; export const SET_LAYOUT_GRIDS_TOOL_NAME = 'set_layout_grids'; @@ -15,23 +16,7 @@ export const setLayoutGridsTool: ToolSpec = { inputSchema: z.object({ nodeId: z.string().describe('Frame (or component/instance) node id'), grids: z - .array( - z.object({ - pattern: z.enum(['GRID', 'ROWS', 'COLUMNS']), - visible: z.boolean(), - sectionSize: z - .number() - .optional() - .describe('Cell size for GRID; section size for ROWS/COLUMNS (ignored when STRETCH)'), - count: z.number().optional().describe('Number of columns/rows (ROWS/COLUMNS)'), - gutterSize: z.number().optional().describe('Gap between columns/rows (ROWS/COLUMNS)'), - alignment: z.enum(['MIN', 'MAX', 'CENTER', 'STRETCH']).optional(), - offset: z - .number() - .optional() - .describe('Page margin from the frame edge (ignored when CENTER)'), - }), - ) + .array(gridItemSchema) .describe('Layout grids to set; [] clears all grids on the frame'), }), kind: 'write', diff --git a/packages/mcp/src/tools/update-effect-style.ts b/packages/mcp/src/tools/update-effect-style.ts index 89e551e..8c5965a 100644 --- a/packages/mcp/src/tools/update-effect-style.ts +++ b/packages/mcp/src/tools/update-effect-style.ts @@ -11,7 +11,9 @@ export const updateEffectStyleTool: ToolSpec = { 'Update an existing effect style by id. Any of name / effects / description may be omitted to ' + 'leave unchanged; effects, when given, replaces the whole list. Shadows (DROP_SHADOW / ' + 'INNER_SHADOW) need color + offset; blurs (LAYER_BLUR / BACKGROUND_BLUR) need radius. Use this ' + - 'to keep a shared style in sync with code instead of creating a duplicate. Returns { ok, ' + + 'to keep a shared style in sync with code instead of creating a duplicate. Because effects ' + + 'replace wholesale, an effect written back WITHOUT its boundVariables clears the variables it ' + + 'was bound to — re-send them, or change the variable itself instead. Returns { ok, ' + 'styleId, name }.', inputSchema: z.object({ styleId: z.string().describe('Effect style id to update'), diff --git a/packages/mcp/src/tools/update-paint-style.ts b/packages/mcp/src/tools/update-paint-style.ts index 4f2e676..c09d790 100644 --- a/packages/mcp/src/tools/update-paint-style.ts +++ b/packages/mcp/src/tools/update-paint-style.ts @@ -9,7 +9,9 @@ export const updatePaintStyleTool: ToolSpec = { name: UPDATE_PAINT_STYLE_TOOL_NAME, description: 'Update an existing paint style by id. Any of name / paints / description may be omitted to ' + - 'leave unchanged. Returns { ok, styleId, name }.', + 'leave unchanged. Because paints replace wholesale, a paint written back WITHOUT its ' + + 'boundVariables clears the variable it was bound to — re-send the binding, or change the ' + + 'variable itself instead. Returns { ok, styleId, name }.', inputSchema: z.object({ styleId: z.string().describe('Paint style id to update'), name: z.string().optional(), diff --git a/packages/mcp/test/plugin-contract.json b/packages/mcp/test/plugin-contract.json index 996eb74..62148ae 100644 --- a/packages/mcp/test/plugin-contract.json +++ b/packages/mcp/test/plugin-contract.json @@ -39,11 +39,13 @@ ], "set_fills": [ "fills", + "fills[].boundVariables", "fills[].color", "fills[].color.b", "fills[].color.g", "fills[].color.r", "fills[].gradientStops", + "fills[].gradientStops[].boundVariables", "fills[].gradientStops[].color", "fills[].gradientStops[].color.a", "fills[].gradientStops[].color.b", @@ -87,11 +89,13 @@ "ranges[].end", "ranges[].fillStyleId", "ranges[].fills", + "ranges[].fills[].boundVariables", "ranges[].fills[].color", "ranges[].fills[].color.b", "ranges[].fills[].color.g", "ranges[].fills[].color.r", "ranges[].fills[].gradientStops", + "ranges[].fills[].gradientStops[].boundVariables", "ranges[].fills[].gradientStops[].color", "ranges[].fills[].gradientStops[].color.a", "ranges[].fills[].gradientStops[].color.b", @@ -151,11 +155,13 @@ "strokeTopWeight", "strokeWeight", "strokes", + "strokes[].boundVariables", "strokes[].color", "strokes[].color.b", "strokes[].color.g", "strokes[].color.r", "strokes[].gradientStops", + "strokes[].gradientStops[].boundVariables", "strokes[].gradientStops[].color", "strokes[].gradientStops[].color.a", "strokes[].gradientStops[].color.b", @@ -207,6 +213,7 @@ "set_layout_grids": [ "grids", "grids[].alignment", + "grids[].boundVariables", "grids[].count", "grids[].gutterSize", "grids[].offset", @@ -226,6 +233,7 @@ "clone_node": ["nodeId", "requestId"], "set_effects": [ "effects", + "effects[].boundVariables", "effects[].color", "effects[].color.a", "effects[].color.b", @@ -245,11 +253,13 @@ "description", "name", "paints", + "paints[].boundVariables", "paints[].color", "paints[].color.b", "paints[].color.g", "paints[].color.r", "paints[].gradientStops", + "paints[].gradientStops[].boundVariables", "paints[].gradientStops[].color", "paints[].gradientStops[].color.a", "paints[].gradientStops[].color.b", @@ -281,6 +291,7 @@ "create_effect_style": [ "description", "effects", + "effects[].boundVariables", "effects[].color", "effects[].color.a", "effects[].color.b", @@ -300,6 +311,7 @@ "description", "grids", "grids[].alignment", + "grids[].boundVariables", "grids[].count", "grids[].gutterSize", "grids[].offset", @@ -313,11 +325,13 @@ "description", "name", "paints", + "paints[].boundVariables", "paints[].color", "paints[].color.b", "paints[].color.g", "paints[].color.r", "paints[].gradientStops", + "paints[].gradientStops[].boundVariables", "paints[].gradientStops[].color", "paints[].gradientStops[].color.a", "paints[].gradientStops[].color.b", @@ -351,6 +365,7 @@ "update_effect_style": [ "description", "effects", + "effects[].boundVariables", "effects[].color", "effects[].color.a", "effects[].color.b", diff --git a/packages/plugin/src/handlers/bindings.ts b/packages/plugin/src/handlers/bindings.ts new file mode 100644 index 0000000..c3e1190 --- /dev/null +++ b/packages/plugin/src/handlers/bindings.ts @@ -0,0 +1,163 @@ +import type { + SerializedBindings, + SerializedEffect, + SerializedLayoutGrid, + SerializedPaint, +} from '@figwright/shared'; + +import { toFigmaEffect, toFigmaLayoutGrid } from './convert.js'; +import { toFigmaPaint } from './set-fills.js'; + +// Write side of the variable bindings serializePaint / serializeEffect / serializeLayoutGrid read +// (issue #164). Without this a read → edit → write round trip replaces a token reference with the +// frozen literal sitting next to it, and the style silently stops tracking its variable. +// +// Figma will happily accept a binding embedded in a plain object literal — that is the only way to +// bind a gradient stop, which has no setter — but measured against a live file, that path validates +// almost nothing: an id matching no variable, and a variable of the wrong resolved type, are both +// taken without complaint and then render as white. So bindings go through the official +// `setBoundVariableFor*` setters wherever one exists, which reject both, and the one case with no +// setter (a gradient stop) is checked here instead. +// +// Each function resolves every id first and only then builds the result, so a bad id throws before +// anything reaches the document rather than leaving a half-written node or a half-built style. + +/** A serialized object's bindings, or undefined when it carries none. */ +const bindingsOf = (src: unknown): SerializedBindings | undefined => { + const raw = (src as { boundVariables?: unknown }).boundVariables; + return typeof raw === 'object' && raw !== null ? (raw as SerializedBindings) : undefined; +}; + +/** + * Every variable id a serialized array references, in the order encountered. Read defensively (the + * idiom serializer.ts uses in the other direction) because only some members of the paint union + * declare `boundVariables` at all — an IMAGE or PATTERN paint has no bindable field. + */ +const idsIn = (sources: readonly unknown[]): string[] => + sources.flatMap(src => Object.values(bindingsOf(src) ?? {})); + +/** + * Resolve ids → Variables, failing loudly on the first that resolves to nothing. Figma does not do + * this for us on the literal path: an unknown id is stored as-is and the value renders white, which + * is exactly the silent breakage this turns into an error. + */ +const resolveVariables = async ( + figmaCtx: typeof figma, + ids: readonly string[], + where: string, +): Promise> => { + const unique = [...new Set(ids)]; + const resolved = await Promise.all( + unique.map(async id => figmaCtx.variables.getVariableByIdAsync(id)), + ); + const table = new Map(); + for (const [i, variable] of resolved.entries()) { + const id = unique[i] as string; + if (variable === null) throw new Error(`${where}: variable ${id} not found`); + table.set(id, variable); + } + return table; +}; + +const variableFor = (table: ReadonlyMap, id: string): Variable => + table.get(id) as Variable; + +/** + * Bind a gradient stop's colour by embedding the alias, the only route Figma offers — + * `setBoundVariableForPaint` takes a SolidPaint. That skips its type check, so make the same one + * here: a non-COLOR variable bound to a stop is accepted silently and paints the stop white. + * Unknown field names need no check; Figma rejects those on assignment even on this path. + */ +const boundStop = ( + stop: ColorStop, + bindings: SerializedBindings, + table: ReadonlyMap, + where: string, +): ColorStop => { + const aliases: Record = {}; + for (const [field, id] of Object.entries(bindings)) { + const variable = variableFor(table, id); + if (field === 'color' && variable.resolvedType !== 'COLOR') { + throw new Error( + `${where}: a gradient stop colour takes a COLOR variable; ${variable.name} is ${variable.resolvedType}`, + ); + } + aliases[field] = { type: 'VARIABLE_ALIAS', id }; + } + return { ...stop, boundVariables: aliases } as ColorStop; +}; + +/** Serialized paints → Figma paints, with each paint's (and each gradient stop's) bindings applied. */ +export const toFigmaPaintsBound = async ( + figmaCtx: typeof figma, + paints: readonly SerializedPaint[], + where: string, +): Promise => { + const stopBindings = paints.flatMap(src => + 'gradientStops' in src ? idsIn(src.gradientStops) : [], + ); + const table = await resolveVariables(figmaCtx, [...idsIn(paints), ...stopBindings], where); + + return paints.map(src => { + const paint = toFigmaPaint(src); + if (src.type === 'SOLID' && src.boundVariables !== undefined) { + let bound = paint as SolidPaint; + for (const [field, id] of Object.entries(src.boundVariables)) { + bound = figmaCtx.variables.setBoundVariableForPaint( + bound, + field as VariableBindablePaintField, + variableFor(table, id), + ); + } + return bound; + } + if ('gradientStops' in src && 'gradientStops' in paint) { + const stops = paint.gradientStops.map((stop, i) => { + const bindings = src.gradientStops[i]?.boundVariables; + return bindings === undefined ? stop : boundStop(stop, bindings, table, where); + }); + return { ...paint, gradientStops: stops } as GradientPaint; + } + return paint; + }); +}; + +/** Serialized effects → Figma effects, with each effect's per-field bindings applied. */ +export const toFigmaEffectsBound = async ( + figmaCtx: typeof figma, + effects: readonly SerializedEffect[], + where: string, +): Promise => { + const table = await resolveVariables(figmaCtx, idsIn(effects), where); + return effects.map(src => { + let effect = toFigmaEffect(src); + for (const [field, id] of Object.entries(src.boundVariables ?? {})) { + effect = figmaCtx.variables.setBoundVariableForEffect( + effect, + field as VariableBindableEffectField, + variableFor(table, id), + ); + } + return effect; + }); +}; + +/** Serialized layout grids → Figma layout grids, with each grid's per-field bindings applied. */ +export const toFigmaLayoutGridsBound = async ( + figmaCtx: typeof figma, + grids: readonly SerializedLayoutGrid[], + where: string, +): Promise => { + const table = await resolveVariables(figmaCtx, idsIn(grids), where); + return grids.map(src => { + let grid = toFigmaLayoutGrid(src); + for (const [field, id] of Object.entries(src.boundVariables ?? {})) { + grid = figmaCtx.variables.setBoundVariableForLayoutGrid( + grid, + field as VariableBindableLayoutGridField, + variableFor(table, id), + ); + } + return grid; + }); +}; diff --git a/packages/plugin/src/handlers/create-effect-style.ts b/packages/plugin/src/handlers/create-effect-style.ts index 06e8730..5b5ce5d 100644 --- a/packages/plugin/src/handlers/create-effect-style.ts +++ b/packages/plugin/src/handlers/create-effect-style.ts @@ -1,7 +1,7 @@ import type { SerializedEffect, StyleResult } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaEffect } from './convert.js'; +import { toFigmaEffectsBound } from './bindings.js'; export const createCreateEffectStyleHandler = (figmaCtx: typeof figma): SandboxToolHandler => @@ -13,9 +13,16 @@ export const createCreateEffectStyleHandler = if (!Array.isArray(p.effects)) throw new TypeError('create_effect_style: effects must be an array'); + // Bindings resolve first, so a bad variable id fails before a half-built style exists. + const effects = await toFigmaEffectsBound( + figmaCtx, + p.effects as SerializedEffect[], + 'create_effect_style', + ); + const style = figmaCtx.createEffectStyle(); style.name = p.name; - style.effects = (p.effects as SerializedEffect[]).map(toFigmaEffect); + style.effects = effects; if (typeof p.description === 'string') style.description = p.description; const result: StyleResult = { ok: true, styleId: style.id, name: style.name }; diff --git a/packages/plugin/src/handlers/create-grid-style.ts b/packages/plugin/src/handlers/create-grid-style.ts index 274f4ea..b82ca15 100644 --- a/packages/plugin/src/handlers/create-grid-style.ts +++ b/packages/plugin/src/handlers/create-grid-style.ts @@ -1,7 +1,7 @@ import type { SerializedLayoutGrid, StyleResult } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaLayoutGrid } from './convert.js'; +import { toFigmaLayoutGridsBound } from './bindings.js'; export const createCreateGridStyleHandler = (figmaCtx: typeof figma): SandboxToolHandler => @@ -11,9 +11,16 @@ export const createCreateGridStyleHandler = if (typeof p.name !== 'string') throw new TypeError('create_grid_style: name must be a string'); if (!Array.isArray(p.grids)) throw new TypeError('create_grid_style: grids must be an array'); + // Bindings resolve first, so a bad variable id fails before a half-built style exists. + const grids = await toFigmaLayoutGridsBound( + figmaCtx, + p.grids as SerializedLayoutGrid[], + 'create_grid_style', + ); + const style = figmaCtx.createGridStyle(); style.name = p.name; - style.layoutGrids = (p.grids as SerializedLayoutGrid[]).map(toFigmaLayoutGrid); + style.layoutGrids = grids; if (typeof p.description === 'string') style.description = p.description; const result: StyleResult = { ok: true, styleId: style.id, name: style.name }; diff --git a/packages/plugin/src/handlers/create-paint-style.ts b/packages/plugin/src/handlers/create-paint-style.ts index 16cfb8b..79cfb14 100644 --- a/packages/plugin/src/handlers/create-paint-style.ts +++ b/packages/plugin/src/handlers/create-paint-style.ts @@ -1,11 +1,10 @@ import type { SerializedPaint, StyleResult } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaPaint } from './set-fills.js'; +import { toFigmaPaintsBound } from './bindings.js'; export const createCreatePaintStyleHandler = (figmaCtx: typeof figma): SandboxToolHandler => - // eslint-disable-next-line @typescript-eslint/require-await async params => { const p = (params ?? {}) as { name?: unknown; paints?: unknown; description?: unknown }; if (typeof p.name !== 'string') @@ -13,9 +12,17 @@ export const createCreatePaintStyleHandler = if (!Array.isArray(p.paints)) throw new TypeError('create_paint_style: paints must be an array'); + // Resolve every binding before creating the style: a bad variable id must not leave a + // half-built style behind in the design-system panel (the create_text_style lesson). + const paints = await toFigmaPaintsBound( + figmaCtx, + p.paints as SerializedPaint[], + 'create_paint_style', + ); + const style = figmaCtx.createPaintStyle(); style.name = p.name; - style.paints = (p.paints as SerializedPaint[]).map(toFigmaPaint); + style.paints = paints; if (typeof p.description === 'string') style.description = p.description; const result: StyleResult = { ok: true, styleId: style.id, name: style.name }; diff --git a/packages/plugin/src/handlers/set-effects.ts b/packages/plugin/src/handlers/set-effects.ts index 3d6bde0..cf01b9f 100644 --- a/packages/plugin/src/handlers/set-effects.ts +++ b/packages/plugin/src/handlers/set-effects.ts @@ -1,7 +1,7 @@ import type { MutateResult, SerializedEffect } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaEffect } from './convert.js'; +import { toFigmaEffectsBound } from './bindings.js'; export const createSetEffectsHandler = (figmaCtx: typeof figma): SandboxToolHandler => @@ -14,7 +14,11 @@ export const createSetEffectsHandler = if (node === null || !('effects' in node)) { throw new Error(`set_effects: node ${p.nodeId} not found or cannot have effects`); } - (node as BlendMixin).effects = (p.effects as SerializedEffect[]).map(toFigmaEffect); + (node as BlendMixin).effects = await toFigmaEffectsBound( + figmaCtx, + p.effects as SerializedEffect[], + 'set_effects', + ); const result: MutateResult = { ok: true, nodeId: node.id }; return result; diff --git a/packages/plugin/src/handlers/set-fills.ts b/packages/plugin/src/handlers/set-fills.ts index c1c74b0..53a67de 100644 --- a/packages/plugin/src/handlers/set-fills.ts +++ b/packages/plugin/src/handlers/set-fills.ts @@ -1,6 +1,7 @@ import type { MutateResult, SerializedPaint } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; +import { toFigmaPaintsBound } from './bindings.js'; /** * Convert a serialized paint back to a Figma Paint. SOLID and the four gradient types are supported @@ -53,7 +54,11 @@ export const createSetFillsHandler = if (node === null || !('fills' in node)) { throw new Error(`set_fills: node ${p.nodeId} not found or cannot have fills`); } - (node as GeometryMixin).fills = (p.fills as SerializedPaint[]).map(toFigmaPaint); + (node as GeometryMixin).fills = await toFigmaPaintsBound( + figmaCtx, + p.fills as SerializedPaint[], + 'set_fills', + ); const result: MutateResult = { ok: true, nodeId: node.id }; return result; diff --git a/packages/plugin/src/handlers/set-layout-grids.ts b/packages/plugin/src/handlers/set-layout-grids.ts index e2936dc..63f1964 100644 --- a/packages/plugin/src/handlers/set-layout-grids.ts +++ b/packages/plugin/src/handlers/set-layout-grids.ts @@ -1,7 +1,7 @@ import type { MutateResult, SerializedLayoutGrid } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaLayoutGrid } from './convert.js'; +import { toFigmaLayoutGridsBound } from './bindings.js'; /** * Replace a frame's own layout grids (its responsive column/row scaffold) — the mirror of the @@ -28,8 +28,10 @@ export const createSetLayoutGridsHandler = ); } - (node as BaseFrameMixin).layoutGrids = (p.grids as SerializedLayoutGrid[]).map( - toFigmaLayoutGrid, + (node as BaseFrameMixin).layoutGrids = await toFigmaLayoutGridsBound( + figmaCtx, + p.grids as SerializedLayoutGrid[], + 'set_layout_grids', ); const result: MutateResult = { ok: true, nodeId: node.id }; diff --git a/packages/plugin/src/handlers/set-strokes.ts b/packages/plugin/src/handlers/set-strokes.ts index caf8644..794611d 100644 --- a/packages/plugin/src/handlers/set-strokes.ts +++ b/packages/plugin/src/handlers/set-strokes.ts @@ -1,7 +1,7 @@ import type { MutateResult, SerializedPaint } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaPaint } from './set-fills.js'; +import { toFigmaPaintsBound } from './bindings.js'; const PER_SIDE = [ 'strokeTopWeight', @@ -39,7 +39,11 @@ export const createSetStrokesHandler = if (node === null || !('strokes' in node)) { throw new Error(`set_strokes: node ${p.nodeId} not found or cannot have strokes`); } - (node as GeometryMixin).strokes = (p.strokes as SerializedPaint[]).map(toFigmaPaint); + (node as GeometryMixin).strokes = await toFigmaPaintsBound( + figmaCtx, + p.strokes as SerializedPaint[], + 'set_strokes', + ); if (typeof p.strokeWeight === 'number') { (node as { strokeWeight: number }).strokeWeight = p.strokeWeight; } diff --git a/packages/plugin/src/handlers/set-text-range.ts b/packages/plugin/src/handlers/set-text-range.ts index 39a197d..250f7dc 100644 --- a/packages/plugin/src/handlers/set-text-range.ts +++ b/packages/plugin/src/handlers/set-text-range.ts @@ -1,8 +1,8 @@ import type { MutateResult, SerializedLineHeight, SerializedPaint } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; +import { toFigmaPaintsBound } from './bindings.js'; import { toFigmaLineHeight } from './convert.js'; -import { toFigmaPaint } from './set-fills.js'; interface RangeInput { start: number; @@ -88,7 +88,15 @@ export const createSetTextRangeHandler = // Direct values first. if (r.fontName !== undefined) text.setRangeFontName(start, end, r.fontName); if (r.fontSize !== undefined) text.setRangeFontSize(start, end, r.fontSize); - if (r.fills !== undefined) text.setRangeFills(start, end, r.fills.map(toFigmaPaint)); + // A run's paints carry their own bindings, exactly like a node's — distinct from the + // range-level `boundVariables` applied below, which binds a field on the run itself. + if (r.fills !== undefined) { + text.setRangeFills( + start, + end, + await toFigmaPaintsBound(figmaCtx, r.fills, 'set_text_range'), + ); + } if (r.textDecoration !== undefined) { text.setRangeTextDecoration(start, end, r.textDecoration as TextDecoration); } diff --git a/packages/plugin/src/handlers/update-effect-style.ts b/packages/plugin/src/handlers/update-effect-style.ts index eba391a..1ff71a2 100644 --- a/packages/plugin/src/handlers/update-effect-style.ts +++ b/packages/plugin/src/handlers/update-effect-style.ts @@ -1,7 +1,7 @@ import type { SerializedEffect, StyleResult } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaEffect } from './convert.js'; +import { toFigmaEffectsBound } from './bindings.js'; export const createUpdateEffectStyleHandler = (figmaCtx: typeof figma): SandboxToolHandler => @@ -22,7 +22,13 @@ export const createUpdateEffectStyleHandler = } const es = style as EffectStyle; if (typeof p.name === 'string') es.name = p.name; - if (Array.isArray(p.effects)) es.effects = (p.effects as SerializedEffect[]).map(toFigmaEffect); + if (Array.isArray(p.effects)) { + es.effects = await toFigmaEffectsBound( + figmaCtx, + p.effects as SerializedEffect[], + 'update_effect_style', + ); + } if (typeof p.description === 'string') es.description = p.description; const result: StyleResult = { ok: true, styleId: es.id, name: es.name }; diff --git a/packages/plugin/src/handlers/update-paint-style.ts b/packages/plugin/src/handlers/update-paint-style.ts index 1e4e7e8..368e7c2 100644 --- a/packages/plugin/src/handlers/update-paint-style.ts +++ b/packages/plugin/src/handlers/update-paint-style.ts @@ -1,7 +1,7 @@ import type { SerializedPaint, StyleResult } from '@figwright/shared'; import type { SandboxToolHandler } from '../dispatcher.js'; -import { toFigmaPaint } from './set-fills.js'; +import { toFigmaPaintsBound } from './bindings.js'; export const createUpdatePaintStyleHandler = (figmaCtx: typeof figma): SandboxToolHandler => @@ -22,7 +22,13 @@ export const createUpdatePaintStyleHandler = } const ps = style as PaintStyle; if (typeof p.name === 'string') ps.name = p.name; - if (Array.isArray(p.paints)) ps.paints = (p.paints as SerializedPaint[]).map(toFigmaPaint); + if (Array.isArray(p.paints)) { + ps.paints = await toFigmaPaintsBound( + figmaCtx, + p.paints as SerializedPaint[], + 'update_paint_style', + ); + } if (typeof p.description === 'string') ps.description = p.description; const result: StyleResult = { ok: true, styleId: ps.id, name: ps.name }; diff --git a/packages/plugin/test/handlers/bindings.test.ts b/packages/plugin/test/handlers/bindings.test.ts new file mode 100644 index 0000000..05aadea --- /dev/null +++ b/packages/plugin/test/handlers/bindings.test.ts @@ -0,0 +1,287 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { + toFigmaEffectsBound, + toFigmaLayoutGridsBound, + toFigmaPaintsBound, +} from '../../src/handlers/bindings.js'; + +// The write half of issue #164. Two measured facts shape every test here: +// +// 1. Figma accepts a binding embedded in a plain object literal — the only way to bind a gradient +// stop, which has no setter — but that path validates almost nothing. An id matching no +// variable, and a variable of the wrong resolved type, are both taken silently and then render +// white. So the official setters are used wherever they exist, and the gradient stop is checked +// here instead. +// 2. Writing an array back WITHOUT `boundVariables` clears whatever it was bound to, because Figma +// replaces the whole array. That is the semantics these functions preserve — there is no +// separate unbind path to test. + +const VAR = (id: string, resolvedType = 'COLOR') => ({ id, name: `var/${id}`, resolvedType }); + +/** SetBoundVariableFor* return a NEW object; the fake records the call and marks the result. */ +const fakeFigma = (variables: Record) => { + const ctx = { + variables: { + getVariableByIdAsync: vi.fn<(id: string) => Promise>( + async (id: string) => variables[id] ?? null, + ), + setBoundVariableForPaint: vi.fn<(p: object, f: string, v: { id: string }) => object>( + (paint: object, field: string, variable: { id: string }) => ({ + ...paint, + boundVariables: { + ...(paint as { boundVariables?: object }).boundVariables, + [field]: { type: 'VARIABLE_ALIAS', id: variable.id }, + }, + }), + ), + setBoundVariableForEffect: vi.fn<(e: object, f: string, v: { id: string }) => object>( + (effect: object, field: string, variable: { id: string }) => ({ + ...effect, + boundVariables: { + ...(effect as { boundVariables?: object }).boundVariables, + [field]: { type: 'VARIABLE_ALIAS', id: variable.id }, + }, + }), + ), + setBoundVariableForLayoutGrid: vi.fn<(g: object, f: string, v: { id: string }) => object>( + (grid: object, field: string, variable: { id: string }) => ({ + ...grid, + boundVariables: { + ...(grid as { boundVariables?: object }).boundVariables, + [field]: { type: 'VARIABLE_ALIAS', id: variable.id }, + }, + }), + ), + }, + }; + return ctx as unknown as typeof figma & { variables: typeof ctx.variables }; +}; + +const solid = (over: Record = {}) => ({ + type: 'SOLID' as const, + visible: true, + opacity: 1, + color: { r: 1, g: 0, b: 0 }, + ...over, +}); + +const gradient = (stops: unknown[]) => ({ + type: 'GRADIENT_LINEAR' as const, + visible: true, + opacity: 1, + gradientTransform: [ + [1, 0, 0], + [0, 1, 0], + ], + gradientStops: stops, +}); + +const shadow = (over: Record = {}) => ({ + type: 'DROP_SHADOW', + visible: true, + radius: 4, + color: { r: 0, g: 0, b: 0, a: 0.2 }, + offset: { x: 0, y: 2 }, + spread: 0, + ...over, +}); + +describe('toFigmaPaintsBound', () => { + it('binds a solid paint through the official setter, not by embedding an alias', async () => { + const ctx = fakeFigma({ 'V:1': VAR('V:1') }); + const out = await toFigmaPaintsBound( + ctx, + [solid({ boundVariables: { color: 'V:1' } })] as never, + 'set_fills', + ); + expect(ctx.variables.setBoundVariableForPaint).toHaveBeenCalledWith( + expect.objectContaining({ type: 'SOLID' }), + 'color', + expect.objectContaining({ id: 'V:1' }), + ); + expect(out[0]).toMatchObject({ + boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } }, + }); + }); + + it('leaves an unbound paint exactly as the plain converter produced it', async () => { + const ctx = fakeFigma({}); + const out = await toFigmaPaintsBound(ctx, [solid()] as never, 'set_fills'); + expect(ctx.variables.setBoundVariableForPaint).not.toHaveBeenCalled(); + expect(ctx.variables.getVariableByIdAsync).not.toHaveBeenCalled(); + expect(out[0]).not.toHaveProperty('boundVariables'); + }); + + it('binds a gradient stop by embedding the alias (Figma exposes no setter for one)', async () => { + const ctx = fakeFigma({ 'V:1': VAR('V:1') }); + const out = (await toFigmaPaintsBound( + ctx, + [ + gradient([ + { position: 0, color: { r: 1, g: 1, b: 1, a: 1 }, boundVariables: { color: 'V:1' } }, + { position: 1, color: { r: 0, g: 0, b: 0, a: 1 } }, + ]), + ] as never, + 'set_fills', + )) as unknown as { gradientStops: { boundVariables?: unknown }[] }[]; + expect(ctx.variables.setBoundVariableForPaint).not.toHaveBeenCalled(); + expect(out[0]?.gradientStops[0]?.boundVariables).toEqual({ + color: { type: 'VARIABLE_ALIAS', id: 'V:1' }, + }); + expect(out[0]?.gradientStops[1]).not.toHaveProperty('boundVariables'); + }); + + it('rejects a non-COLOR variable on a gradient stop, which Figma would accept silently', async () => { + const ctx = fakeFigma({ 'V:n': VAR('V:n', 'FLOAT') }); + await expect( + toFigmaPaintsBound( + ctx, + [ + gradient([ + { position: 0, color: { r: 1, g: 1, b: 1, a: 1 }, boundVariables: { color: 'V:n' } }, + ]), + ] as never, + 'set_fills', + ), + ).rejects.toThrow(/gradient stop colour takes a COLOR variable; var\/V:n is FLOAT/); + }); + + it('rejects an id that resolves to no variable', async () => { + const ctx = fakeFigma({}); + await expect( + toFigmaPaintsBound( + ctx, + [solid({ boundVariables: { color: 'V:gone' } })] as never, + 'set_fills', + ), + ).rejects.toThrow('set_fills: variable V:gone not found'); + }); + + it('resolves before binding anything, so a bad id in a later paint aborts the whole array', async () => { + const ctx = fakeFigma({ 'V:1': VAR('V:1') }); + await expect( + toFigmaPaintsBound( + ctx, + [ + solid({ boundVariables: { color: 'V:1' } }), + solid({ boundVariables: { color: 'V:no' } }), + ] as never, + 'set_fills', + ), + ).rejects.toThrow('variable V:no not found'); + // The good paint must not have been bound either — the caller assigns the returned array, so a + // partial result would be a half-written node. + expect(ctx.variables.setBoundVariableForPaint).not.toHaveBeenCalled(); + }); + + it('looks each distinct variable up once, however many paints cite it', async () => { + const ctx = fakeFigma({ 'V:1': VAR('V:1') }); + await toFigmaPaintsBound( + ctx, + [ + solid({ boundVariables: { color: 'V:1' } }), + solid({ boundVariables: { color: 'V:1' } }), + gradient([ + { position: 0, color: { r: 0, g: 0, b: 0, a: 1 }, boundVariables: { color: 'V:1' } }, + ]), + ] as never, + 'set_fills', + ); + expect(ctx.variables.getVariableByIdAsync).toHaveBeenCalledTimes(1); + }); +}); + +describe('toFigmaEffectsBound', () => { + it('applies every bound field, chaining the new object each setter returns', async () => { + const ctx = fakeFigma({ 'V:c': VAR('V:c'), 'V:n': VAR('V:n', 'FLOAT') }); + const out = await toFigmaEffectsBound( + ctx, + [shadow({ boundVariables: { color: 'V:c', radius: 'V:n' } })] as never, + 'set_effects', + ); + expect(ctx.variables.setBoundVariableForEffect).toHaveBeenCalledTimes(2); + // Chained, not applied to the original: both bindings survive on one object. + expect(out[0]).toMatchObject({ + boundVariables: { + color: { type: 'VARIABLE_ALIAS', id: 'V:c' }, + radius: { type: 'VARIABLE_ALIAS', id: 'V:n' }, + }, + }); + }); + + it('passes the field through untranslated, so a newly bindable field needs no change here', async () => { + const ctx = fakeFigma({ 'V:n': VAR('V:n', 'FLOAT') }); + await toFigmaEffectsBound( + ctx, + [shadow({ boundVariables: { offsetX: 'V:n' } })] as never, + 'set_effects', + ); + expect(ctx.variables.setBoundVariableForEffect).toHaveBeenCalledWith( + expect.anything(), + 'offsetX', + expect.objectContaining({ id: 'V:n' }), + ); + }); + + it('rejects an unresolvable id before touching the setter', async () => { + const ctx = fakeFigma({}); + await expect( + toFigmaEffectsBound( + ctx, + [shadow({ boundVariables: { color: 'V:x' } })] as never, + 'set_effects', + ), + ).rejects.toThrow('set_effects: variable V:x not found'); + expect(ctx.variables.setBoundVariableForEffect).not.toHaveBeenCalled(); + }); +}); + +describe('toFigmaLayoutGridsBound', () => { + it('binds a grid field through the official setter', async () => { + const ctx = fakeFigma({ 'V:n': VAR('V:n', 'FLOAT') }); + const out = await toFigmaLayoutGridsBound( + ctx, + [ + { + pattern: 'COLUMNS', + visible: true, + count: 12, + gutterSize: 16, + alignment: 'STRETCH', + boundVariables: { gutterSize: 'V:n' }, + }, + ] as never, + 'set_layout_grids', + ); + expect(ctx.variables.setBoundVariableForLayoutGrid).toHaveBeenCalledWith( + expect.objectContaining({ pattern: 'COLUMNS' }), + 'gutterSize', + expect.objectContaining({ id: 'V:n' }), + ); + expect(out[0]).toMatchObject({ + boundVariables: { gutterSize: { type: 'VARIABLE_ALIAS', id: 'V:n' } }, + }); + }); + + it('binds on the uniform GRID pattern too', async () => { + const ctx = fakeFigma({ 'V:n': VAR('V:n', 'FLOAT') }); + await toFigmaLayoutGridsBound( + ctx, + [ + { + pattern: 'GRID', + visible: true, + sectionSize: 8, + boundVariables: { sectionSize: 'V:n' }, + }, + ] as never, + 'set_layout_grids', + ); + expect(ctx.variables.setBoundVariableForLayoutGrid).toHaveBeenCalledWith( + expect.objectContaining({ pattern: 'GRID' }), + 'sectionSize', + expect.anything(), + ); + }); +}); diff --git a/packages/plugin/test/handlers/create-effect-style.test.ts b/packages/plugin/test/handlers/create-effect-style.test.ts index d896d0f..36d0246 100644 --- a/packages/plugin/test/handlers/create-effect-style.test.ts +++ b/packages/plugin/test/handlers/create-effect-style.test.ts @@ -3,10 +3,28 @@ import { describe, expect, it } from 'vitest'; import { createCreateEffectStyleHandler } from '../../src/handlers/create-effect-style.js'; -const fakeFigma = (): { figma: typeof figma; style: Record } => { +const fakeFigma = ( + variables: Record = {}, +): { figma: typeof figma; style: Record; creations: () => number } => { const style: Record = { id: 'E:0', name: '' }; - const figmaCtx = { createEffectStyle: () => style } as unknown as typeof figma; - return { figma: figmaCtx, style }; + let created = 0; + const figmaCtx = { + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForEffect: (effect: object, field: string, v: { id: string }) => ({ + ...effect, + boundVariables: { + ...(effect as { boundVariables?: object }).boundVariables, + [field]: { type: 'VARIABLE_ALIAS', id: v.id }, + }, + }), + }, + createEffectStyle: () => { + created += 1; + return style; + }, + } as unknown as typeof figma; + return { figma: figmaCtx, style, creations: () => created }; }; describe('create_effect_style handler', () => { @@ -28,4 +46,47 @@ describe('create_effect_style handler', () => { await expect(handler({ effects: [] })).rejects.toThrow(/name/); await expect(handler({ name: 'x', effects: 'no' })).rejects.toThrow(/effects/); }); + + // Same rule as create_paint_style: resolve the bindings first, or a bad id leaves a style behind. + it('creates nothing when a bound variable cannot be resolved', async () => { + const { figma: f, creations } = fakeFigma(); + await expect( + createCreateEffectStyleHandler(f)({ + name: 'Elevation/focus', + effects: [ + { + type: 'DROP_SHADOW', + visible: true, + radius: 4, + color: { r: 0, g: 0, b: 0, a: 0.2 }, + offset: { x: 0, y: 2 }, + boundVariables: { color: 'V:gone' }, + }, + ], + }), + ).rejects.toThrow('create_effect_style: variable V:gone not found'); + expect(creations()).toBe(0); + }); + + it('carries a shadow binding onto the created style', async () => { + const { figma: f, style } = fakeFigma({ + 'V:1': { id: 'V:1', name: 'color/information', resolvedType: 'COLOR' }, + }); + await createCreateEffectStyleHandler(f)({ + name: 'Elevation/focus', + effects: [ + { + type: 'DROP_SHADOW', + visible: true, + radius: 4, + color: { r: 0, g: 0, b: 0, a: 0.2 }, + offset: { x: 0, y: 2 }, + boundVariables: { color: 'V:1' }, + }, + ], + }); + expect(style.effects).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/create-grid-style.test.ts b/packages/plugin/test/handlers/create-grid-style.test.ts index 6084b89..f4ffcba 100644 --- a/packages/plugin/test/handlers/create-grid-style.test.ts +++ b/packages/plugin/test/handlers/create-grid-style.test.ts @@ -3,10 +3,25 @@ import { describe, expect, it } from 'vitest'; import { createCreateGridStyleHandler } from '../../src/handlers/create-grid-style.js'; -const fakeFigma = (): { figma: typeof figma; style: Record } => { +const fakeFigma = ( + variables: Record = {}, +): { figma: typeof figma; style: Record; creations: () => number } => { const style: Record = { id: 'G:0', name: '' }; - const figmaCtx = { createGridStyle: () => style } as unknown as typeof figma; - return { figma: figmaCtx, style }; + let created = 0; + const figmaCtx = { + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForLayoutGrid: (grid: object, field: string, v: { id: string }) => ({ + ...grid, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + createGridStyle: () => { + created += 1; + return style; + }, + } as unknown as typeof figma; + return { figma: figmaCtx, style, creations: () => created }; }; describe('create_grid_style handler', () => { @@ -28,4 +43,46 @@ describe('create_grid_style handler', () => { await expect(handler({ grids: [] })).rejects.toThrow(/name/); await expect(handler({ name: 'x', grids: 'no' })).rejects.toThrow(/grids/); }); + + it('creates nothing when a bound variable cannot be resolved', async () => { + const { figma: f, creations } = fakeFigma(); + await expect( + createCreateGridStyleHandler(f)({ + name: 'Layout/8pt', + grids: [ + { + pattern: 'GRID', + visible: true, + sectionSize: 8, + boundVariables: { sectionSize: 'V:x' }, + }, + ], + }), + ).rejects.toThrow('create_grid_style: variable V:x not found'); + expect(creations()).toBe(0); + }); + + it('carries a grid binding onto the created style', async () => { + const { figma: f, style } = fakeFigma({ + 'V:1': { id: 'V:1', name: 'size/gutter', resolvedType: 'FLOAT' }, + }); + await createCreateGridStyleHandler(f)({ + name: 'Layout/12col', + grids: [ + { + pattern: 'COLUMNS', + visible: true, + count: 12, + gutterSize: 16, + alignment: 'STRETCH', + boundVariables: { gutterSize: 'V:1' }, + }, + ], + }); + expect(style.layoutGrids).toEqual([ + expect.objectContaining({ + boundVariables: { gutterSize: { type: 'VARIABLE_ALIAS', id: 'V:1' } }, + }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/create-paint-style.test.ts b/packages/plugin/test/handlers/create-paint-style.test.ts index 4f912c5..23f10a9 100644 --- a/packages/plugin/test/handlers/create-paint-style.test.ts +++ b/packages/plugin/test/handlers/create-paint-style.test.ts @@ -10,9 +10,18 @@ interface FakePaintStyle { description: string; } -const fakeFigma = (): { figma: typeof figma; created: FakePaintStyle[] } => { +const fakeFigma = ( + variables: Record = {}, +): { figma: typeof figma; created: FakePaintStyle[] } => { const created: FakePaintStyle[] = []; const figmaCtx = { + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForPaint: (paint: object, field: string, v: { id: string }) => ({ + ...paint, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, createPaintStyle: () => { const style: FakePaintStyle = { id: `S:${created.length}`, @@ -51,4 +60,48 @@ describe('create_paint_style handler', () => { await expect(handler({ paints: [] })).rejects.toThrow(/name/); await expect(handler({ name: 'x', paints: 'nope' })).rejects.toThrow(/paints/); }); + + // A style whose paints cite a variable that no longer exists must fail before the style exists: + // Figma has no transaction, so a style created first would be left behind in the design-system + // panel — and published to the library from there (the create_text_style lesson). + it('creates nothing when a bound variable cannot be resolved', async () => { + const { figma: f, created } = fakeFigma(); + const handler = createCreatePaintStyleHandler(f); + await expect( + handler({ + name: 'Brand/Primary', + paints: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 1, g: 0, b: 0 }, + boundVariables: { color: 'V:gone' }, + }, + ], + }), + ).rejects.toThrow('create_paint_style: variable V:gone not found'); + expect(created).toEqual([]); + }); + + it('carries a paint binding onto the created style', async () => { + const { figma: f, created } = fakeFigma({ + 'V:1': { id: 'V:1', name: 'color/brand', resolvedType: 'COLOR' }, + }); + await createCreatePaintStyleHandler(f)({ + name: 'Brand/Primary', + paints: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 1, g: 0, b: 0 }, + boundVariables: { color: 'V:1' }, + }, + ], + }); + expect(created[0]?.paints).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/set-effects.test.ts b/packages/plugin/test/handlers/set-effects.test.ts index 9fbf95c..580a699 100644 --- a/packages/plugin/test/handlers/set-effects.test.ts +++ b/packages/plugin/test/handlers/set-effects.test.ts @@ -3,8 +3,20 @@ import { describe, expect, it } from 'vitest'; import { createSetEffectsHandler } from '../../src/handlers/set-effects.js'; -const fakeFigma = (lookup: Record): typeof figma => - ({ getNodeByIdAsync: async (id: string) => lookup[id] ?? null }) as unknown as typeof figma; +const fakeFigma = ( + lookup: Record, + variables: Record = {}, +): typeof figma => + ({ + getNodeByIdAsync: async (id: string) => lookup[id] ?? null, + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForEffect: (effect: object, field: string, v: { id: string }) => ({ + ...effect, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + }) as unknown as typeof figma; const dropShadow = { type: 'DROP_SHADOW', @@ -45,4 +57,18 @@ describe('set_effects handler', () => { handler({ nodeId: '1:1', effects: [{ type: 'DROP_SHADOW', visible: true }] }), ).rejects.toThrow(/color and offset/); }); + + it('applies a shadow binding, so a token shadow colour stays a token', async () => { + const node = { id: '1:1', effects: [] as unknown }; + const handler = createSetEffectsHandler( + fakeFigma({ '1:1': node }, { 'V:1': { id: 'V:1', name: 'token', resolvedType: 'COLOR' } }), + ); + await handler({ + nodeId: '1:1', + effects: [{ ...dropShadow, boundVariables: { color: 'V:1' } }], + }); + expect(node.effects).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/set-fills.test.ts b/packages/plugin/test/handlers/set-fills.test.ts index cce370f..7d1cc1b 100644 --- a/packages/plugin/test/handlers/set-fills.test.ts +++ b/packages/plugin/test/handlers/set-fills.test.ts @@ -3,8 +3,20 @@ import { describe, expect, it } from 'vitest'; import { createSetFillsHandler } from '../../src/handlers/set-fills.js'; -const fakeFigma = (lookup: Record): typeof figma => - ({ getNodeByIdAsync: async (id: string) => lookup[id] ?? null }) as unknown as typeof figma; +const fakeFigma = ( + lookup: Record, + variables: Record = {}, +): typeof figma => + ({ + getNodeByIdAsync: async (id: string) => lookup[id] ?? null, + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForPaint: (paint: object, field: string, v: { id: string }) => ({ + ...paint, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + }) as unknown as typeof figma; describe('set_fills handler', () => { it('applies SOLID fills to a node and returns ok + nodeId', async () => { @@ -68,4 +80,48 @@ describe('set_fills handler', () => { await expect(handler({ fills: [] })).rejects.toThrow(/nodeId/); await expect(handler({ nodeId: '1:1', fills: 'x' })).rejects.toThrow(/fills/); }); + + // A fill's variable binding lives on the paint (issue #164). Writing the paint back without + // applying it would silently replace the token with the literal beside it. + it('applies a paint binding, so a token fill stays a token', async () => { + const node = { id: '1:1', fills: [] as unknown }; + const handler = createSetFillsHandler( + fakeFigma({ '1:1': node }, { 'V:1': { id: 'V:1', name: 'token', resolvedType: 'COLOR' } }), + ); + await handler({ + nodeId: '1:1', + fills: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 1, g: 0, b: 0 }, + boundVariables: { color: 'V:1' }, + }, + ], + }); + expect(node.fills).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); + + it('leaves the node untouched when a bound variable cannot be resolved', async () => { + const node = { id: '1:1', fills: 'UNTOUCHED' as unknown }; + const handler = createSetFillsHandler(fakeFigma({ '1:1': node })); + await expect( + handler({ + nodeId: '1:1', + fills: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 1, g: 0, b: 0 }, + boundVariables: { color: 'V:gone' }, + }, + ], + }), + ).rejects.toThrow('set_fills: variable V:gone not found'); + expect(node.fills).toBe('UNTOUCHED'); + }); }); diff --git a/packages/plugin/test/handlers/set-layout-grids.test.ts b/packages/plugin/test/handlers/set-layout-grids.test.ts index af84d75..6b67b3b 100644 --- a/packages/plugin/test/handlers/set-layout-grids.test.ts +++ b/packages/plugin/test/handlers/set-layout-grids.test.ts @@ -3,8 +3,20 @@ import { describe, expect, it } from 'vitest'; import { createSetLayoutGridsHandler } from '../../src/handlers/set-layout-grids.js'; -const fakeFigma = (lookup: Record): typeof figma => - ({ getNodeByIdAsync: async (id: string) => lookup[id] ?? null }) as unknown as typeof figma; +const fakeFigma = ( + lookup: Record, + variables: Record = {}, +): typeof figma => + ({ + getNodeByIdAsync: async (id: string) => lookup[id] ?? null, + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForLayoutGrid: (grid: object, field: string, v: { id: string }) => ({ + ...grid, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + }) as unknown as typeof figma; describe('set_layout_grids handler', () => { it('sets a 12-column grid on a frame (the responsive column scaffold)', async () => { @@ -92,4 +104,29 @@ describe('set_layout_grids handler', () => { await expect(handler({ nodeId: '9:9', grids: [] })).rejects.toThrow(/not found/); await expect(handler({ nodeId: '2:2', grids: [] })).rejects.toThrow(/does not support/); }); + + it('applies a grid binding, so a token gutter stays a token', async () => { + const frame = { id: '1:1', layoutGrids: [] as unknown[] }; + const handler = createSetLayoutGridsHandler( + fakeFigma({ '1:1': frame }, { 'V:1': { id: 'V:1', name: 'token', resolvedType: 'FLOAT' } }), + ); + await handler({ + nodeId: '1:1', + grids: [ + { + pattern: 'COLUMNS', + visible: true, + count: 12, + gutterSize: 24, + alignment: 'STRETCH', + boundVariables: { gutterSize: 'V:1' }, + }, + ], + }); + expect(frame.layoutGrids).toEqual([ + expect.objectContaining({ + boundVariables: { gutterSize: { type: 'VARIABLE_ALIAS', id: 'V:1' } }, + }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/set-strokes.test.ts b/packages/plugin/test/handlers/set-strokes.test.ts index feac250..465c933 100644 --- a/packages/plugin/test/handlers/set-strokes.test.ts +++ b/packages/plugin/test/handlers/set-strokes.test.ts @@ -3,8 +3,20 @@ import { describe, expect, it } from 'vitest'; import { createSetStrokesHandler } from '../../src/handlers/set-strokes.js'; -const fakeFigma = (lookup: Record): typeof figma => - ({ getNodeByIdAsync: async (id: string) => lookup[id] ?? null }) as unknown as typeof figma; +const fakeFigma = ( + lookup: Record, + variables: Record = {}, +): typeof figma => + ({ + getNodeByIdAsync: async (id: string) => lookup[id] ?? null, + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForPaint: (paint: object, field: string, v: { id: string }) => ({ + ...paint, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + }) as unknown as typeof figma; describe('set_strokes handler', () => { it('applies SOLID strokes and strokeWeight', async () => { @@ -69,4 +81,26 @@ describe('set_strokes handler', () => { ); await expect(handler({ nodeId: '9:9', strokes: [] })).rejects.toThrow(/not found/); }); + + it('applies a paint binding to a stroke', async () => { + const node = { id: '1:1', strokes: [] as unknown, strokeWeight: 0 }; + const handler = createSetStrokesHandler( + fakeFigma({ '1:1': node }, { 'V:1': { id: 'V:1', name: 'token', resolvedType: 'COLOR' } }), + ); + await handler({ + nodeId: '1:1', + strokes: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 0, g: 0, b: 0 }, + boundVariables: { color: 'V:1' }, + }, + ], + }); + expect(node.strokes).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/set-text-range.test.ts b/packages/plugin/test/handlers/set-text-range.test.ts index 873d962..6a7f8ec 100644 --- a/packages/plugin/test/handlers/set-text-range.test.ts +++ b/packages/plugin/test/handlers/set-text-range.test.ts @@ -230,4 +230,60 @@ describe('set_text_range handler', () => { }), ).rejects.toThrow(/variable VariableID:missing not found/); }); + + // A run's paints carry bindings of their own, distinct from the range-level `boundVariables` + // below them — the same shape get_node reports for a mixed TEXT node's segments. Before #164's + // write half they were dropped here, which turned a token-coloured word into a frozen hex. + it('carries a binding on a run fill through to setRangeFills', async () => { + const text = makeText(); + const variable = { id: 'V:1', name: 'color/link', resolvedType: 'COLOR' }; + const figma = makeFigma(text, { 'V:1': variable }); + await createSetTextRangeHandler(figma)({ + nodeId: 'T:1', + ranges: [ + { + start: 0, + end: 5, + fills: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 0, g: 0, b: 1 }, + boundVariables: { color: 'V:1' }, + }, + ], + }, + ], + }); + expect(text.setRangeFills).toHaveBeenCalledWith(0, 5, [ + expect.objectContaining({ boundVariables: { color: variable } }), + ]); + }); + + it('rejects a run fill bound to a variable that no longer exists', async () => { + const text = makeText(); + const figma = makeFigma(text); + await expect( + createSetTextRangeHandler(figma)({ + nodeId: 'T:1', + ranges: [ + { + start: 0, + end: 5, + fills: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 0, g: 0, b: 1 }, + boundVariables: { color: 'V:gone' }, + }, + ], + }, + ], + }), + ).rejects.toThrow('set_text_range: variable V:gone not found'); + expect(text.setRangeFills).not.toHaveBeenCalled(); + }); }); diff --git a/packages/plugin/test/handlers/update-effect-style.test.ts b/packages/plugin/test/handlers/update-effect-style.test.ts index f9833f3..3b2358f 100644 --- a/packages/plugin/test/handlers/update-effect-style.test.ts +++ b/packages/plugin/test/handlers/update-effect-style.test.ts @@ -3,8 +3,17 @@ import { describe, expect, it } from 'vitest'; import { createUpdateEffectStyleHandler } from '../../src/handlers/update-effect-style.js'; -const fakeFigma = (style: unknown): typeof figma => - ({ getStyleByIdAsync: async () => style }) as unknown as typeof figma; +const fakeFigma = (style: unknown, variables: Record = {}): typeof figma => + ({ + getStyleByIdAsync: async () => style, + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForEffect: (effect: object, field: string, v: { id: string }) => ({ + ...effect, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + }) as unknown as typeof figma; describe('update_effect_style handler', () => { it('replaces effects + name and leaves an omitted field unchanged', async () => { @@ -36,4 +45,26 @@ describe('update_effect_style handler', () => { ).rejects.toThrow(/not found/); await expect(createUpdateEffectStyleHandler(fakeFigma(null))({})).rejects.toThrow(/styleId/); }); + + it('re-applies a binding sent with the new effects', async () => { + const style = { id: 'E:0', type: 'EFFECT', name: 'n', effects: [] as unknown, description: '' }; + await createUpdateEffectStyleHandler( + fakeFigma(style, { 'V:1': { id: 'V:1', name: 'token', resolvedType: 'COLOR' } }), + )({ + styleId: 'E:0', + effects: [ + { + type: 'DROP_SHADOW', + visible: true, + radius: 4, + color: { r: 0, g: 0, b: 0, a: 0.2 }, + offset: { x: 0, y: 2 }, + boundVariables: { color: 'V:1' }, + }, + ], + }); + expect(style.effects).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); }); diff --git a/packages/plugin/test/handlers/update-paint-style.test.ts b/packages/plugin/test/handlers/update-paint-style.test.ts index e28b179..a6e85b0 100644 --- a/packages/plugin/test/handlers/update-paint-style.test.ts +++ b/packages/plugin/test/handlers/update-paint-style.test.ts @@ -3,8 +3,17 @@ import { describe, expect, it } from 'vitest'; import { createUpdatePaintStyleHandler } from '../../src/handlers/update-paint-style.js'; -const fakeFigma = (style: unknown): typeof figma => - ({ getStyleByIdAsync: async () => style }) as unknown as typeof figma; +const fakeFigma = (style: unknown, variables: Record = {}): typeof figma => + ({ + getStyleByIdAsync: async () => style, + variables: { + getVariableByIdAsync: async (id: string) => variables[id] ?? null, + setBoundVariableForPaint: (paint: object, field: string, v: { id: string }) => ({ + ...paint, + boundVariables: { [field]: { type: 'VARIABLE_ALIAS', id: v.id } }, + }), + }, + }) as unknown as typeof figma; describe('update_paint_style handler', () => { it('updates name + paints of an existing paint style', async () => { @@ -32,4 +41,46 @@ describe('update_paint_style handler', () => { ).rejects.toThrow(/not found/); await expect(createUpdatePaintStyleHandler(fakeFigma(null))({})).rejects.toThrow(/styleId/); }); + + // Re-syncing a style from code is the common case, and it is where a binding gets clobbered: + // paints replace wholesale, so the binding has to be re-sent — and then actually applied. + it('re-applies a binding sent with the new paints', async () => { + const style = { id: 'S:0', type: 'PAINT', name: 'n', paints: [] as unknown, description: '' }; + await createUpdatePaintStyleHandler( + fakeFigma(style, { 'V:1': { id: 'V:1', name: 'token', resolvedType: 'COLOR' } }), + )({ + styleId: 'S:0', + paints: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 0, g: 1, b: 0 }, + boundVariables: { color: 'V:1' }, + }, + ], + }); + expect(style.paints).toEqual([ + expect.objectContaining({ boundVariables: { color: { type: 'VARIABLE_ALIAS', id: 'V:1' } } }), + ]); + }); + + it('leaves the style untouched when a bound variable cannot be resolved', async () => { + const style = { id: 'S:0', type: 'PAINT', name: 'n', paints: 'UNTOUCHED', description: '' }; + await expect( + createUpdatePaintStyleHandler(fakeFigma(style))({ + styleId: 'S:0', + paints: [ + { + type: 'SOLID', + visible: true, + opacity: 1, + color: { r: 0, g: 1, b: 0 }, + boundVariables: { color: 'V:gone' }, + }, + ], + }), + ).rejects.toThrow('update_paint_style: variable V:gone not found'); + expect(style.paints).toBe('UNTOUCHED'); + }); }); diff --git a/skills/figma-build/references/author-design-system.md b/skills/figma-build/references/author-design-system.md index 6022517..3005099 100644 --- a/skills/figma-build/references/author-design-system.md +++ b/skills/figma-build/references/author-design-system.md @@ -59,15 +59,22 @@ an orphan collection behind. `styleId`s to target. - **A value the style binds to a variable is a reference, not a literal.** `get_styles` reports those bindings as `boundVariables` (`{ field: variableId }`) on the paint / effect / layout grid that - owns them — and on the text style itself for typography — with `variables` naming each id. Writing - that value back as a literal through `update_*` replaces the reference with a frozen copy, so the - style silently stops tracking the token it was built on. When code changes such a value, change the - **variable** (`set_variable_value`) instead; only rewrite the style's own paints/effects when the - field genuinely carries no binding. + owns them — and on the text style itself for typography — with `variables` naming each id. Paints + and effects **replace wholesale**, so an `update_*` that omits `boundVariables` replaces the + reference with a frozen copy and the style silently stops tracking its token. Carry the binding + through: send the paint/effect back **with** its `boundVariables`. And when what actually changed + is the token's value, change the **variable** (`set_variable_value`) rather than the style. Apply a style to a node with `apply_style_to_node` (`field`: fill / stroke / effect / grid / text). Prefer a **variable** for a single colour/scalar token and a **style** for a reusable multi-property -look (a shadow, a type ramp step). +look (a shadow, a type ramp step). The two compose rather than compete: a style's own values can be +**bound to variables**, which is how a shadow tracks a colour token or a grid tracks a spacing one. +Bind by including `boundVariables` (`{ field: variableId }`) on the paint / effect / layout grid you +pass to `create_*` / `update_*` / `set_fills` / `set_strokes` / `set_effects` / `set_layout_grids` — +`color` on a SOLID paint or a gradient stop, `color` / `radius` / `spread` / `offsetX` / `offsetY` +on a shadow, `sectionSize` / `count` / `offset` / `gutterSize` on a grid. A binding whose variable +id does not resolve is an error, not a silent miss, so a stale id fails loudly instead of painting +the value white. ## Components & variant sets