fix(fig): harden component property round trips

- Allocate collision-safe property GUIDs across every export path
- Preserve instance-swap component keys and unresolved GUID targets
- Prefer edited assignments over imported raw metadata
- Cover recursive serialization, assignments, and key preservation
This commit is contained in:
Danila Poyarkov 2026-08-19 18:05:25 +03:00
parent 8aeda81f3a
commit 33962b764c
9 changed files with 257 additions and 39 deletions

View file

@ -58,6 +58,7 @@
### Fixed
- Preserve app-created component properties and instance-swap targets across `.fig` save and reload cycles. (#548)
- Reconnect desktop automation to an already-running MCP server by allowing access to its discovery file. (#546)
- Keep text-editing carets, hit testing, and selection highlights aligned with vertically centered or bottom-aligned text. (#539)
- Match AI chat code-block syntax colors and backgrounds to the active light or dark theme. (#537)

View file

@ -163,6 +163,51 @@ function assignVariableGuids(
}
}
interface ComponentPropertyGuidState {
ids: string[]
maxLocalId0: number
maxLocalId1: number
}
function collectComponentPropertyGuidState(graph: SceneGraph): ComponentPropertyGuidState {
const ids = new Set<string>()
let maxLocalId0 = 0
let maxLocalId1 = 0
for (const node of graph.getAllNodes()) {
for (const definition of node.componentPropertyDefinitions) ids.add(definition.id)
for (const reference of node.componentPropertyReferences) ids.add(reference.propertyId)
for (const propertyId of Object.keys(node.componentPropertyAssignments)) ids.add(propertyId)
for (const spec of node.variantPropSpecs) ids.add(spec.propDefId)
}
for (const propertyId of ids) {
const match = /^(\d+):(\d+)$/.exec(propertyId)
if (!match) continue
const sessionID = Number.parseInt(match[1], 10)
const localID = Number.parseInt(match[2], 10)
if (sessionID === 0) maxLocalId0 = Math.max(maxLocalId0, localID)
if (sessionID === 1) maxLocalId1 = Math.max(maxLocalId1, localID)
}
return { ids: [...ids], maxLocalId0, maxLocalId1 }
}
function assignComponentPropertyGuids(
propertyIds: readonly string[],
localIdCounter: { value: number },
propertyIdToGuid: Map<string, GUID>,
assignedGuidValues: Set<string>,
nodeSourceGuidValues: Set<string>
): void {
for (const propertyId of propertyIds) {
const guid = assignVariableGuid(
propertyId,
localIdCounter,
assignedGuidValues,
nodeSourceGuidValues
)
propertyIdToGuid.set(propertyId, guid)
}
}
function appendVariableNodeChanges(
graph: SceneGraph,
nodeChanges: KiwiNodeChange[],
@ -466,6 +511,9 @@ export async function exportFigFile(
}
}
}
const propertyGuidState = collectComponentPropertyGuidState(graph)
maxLocalId0 = Math.max(maxLocalId0, propertyGuidState.maxLocalId0)
maxLocalId1 = Math.max(maxLocalId1, propertyGuidState.maxLocalId1)
localIdCounter.value = Math.max(localIdCounter.value, maxLocalId0 + 1, maxLocalId1 + 1)
const { canvasEntries, internalCanvasGuid } = buildCanvasEntries(
@ -488,6 +536,14 @@ export async function exportFigFile(
nodeSourceGuidValues
)
assignComponentPropertyGuids(
propertyGuidState.ids,
localIdCounter,
propertyIdToGuid,
assignedGuidValues,
nodeSourceGuidValues
)
for (const entry of canvasEntries) nodeChanges.push(entry.canvasNc)
const orderedCanvasEntries = [

View file

@ -407,7 +407,10 @@ function remapComponentIds(graph: SceneGraph, guidToNodeId: Map<string, string>)
* how it was exported), not this import's freshly-assigned node ID — remap
* them the same way remapComponentIds fixes up instance.componentId.
*/
function remapInstanceSwapPropertyValues(graph: SceneGraph, guidToNodeId: Map<string, string>): void {
function remapInstanceSwapPropertyValues(
graph: SceneGraph,
guidToNodeId: Map<string, string>
): void {
const defsById = new Map<string, ComponentPropertyDefinition>()
for (const node of graph.getAllNodes()) {
for (const def of node.componentPropertyDefinitions) {
@ -421,13 +424,8 @@ function remapInstanceSwapPropertyValues(graph: SceneGraph, guidToNodeId: Map<st
const defs = node.componentPropertyDefinitions.map((def) => {
if (def.type !== 'INSTANCE_SWAP') return def
const remappedDefault = def.defaultValue ? guidToNodeId.get(def.defaultValue) : undefined
const remappedPreferred = def.preferredValues?.map((value) => guidToNodeId.get(value) ?? value)
if (!remappedDefault && !remappedPreferred) return def
return {
...def,
defaultValue: remappedDefault ?? def.defaultValue,
preferredValues: remappedPreferred ?? def.preferredValues
}
if (!remappedDefault) return def
return { ...def, defaultValue: remappedDefault }
})
const changed = defs.some((def, i) => def !== node.componentPropertyDefinitions[i])
if (changed) graph.updateNode(node.id, { componentPropertyDefinitions: defs })

View file

@ -1,5 +1,5 @@
import type { NodeChange, Paint } from '@open-pencil/kiwi/fig/codec'
import { guidToString, stringToGuid } from '@open-pencil/kiwi/fig/guid'
import { stringToGuid } from '@open-pencil/kiwi/fig/guid'
import { DEFAULT_STROKE_MITER_LIMIT } from '@open-pencil/scene-graph'
import type {
ComponentPropertyDefinition,
@ -68,7 +68,7 @@ interface SceneNodeToKiwiContext {
/** GUIDs minted for component property IDs (e.g. "prop:abc123") that aren't
* already Figma-GUID-shaped, keyed by the original ID so refs/assignments/
* variantPropSpecs pointing at the same property reuse the same GUID. */
propertyIdToGuid?: Map<string, GUID>
propertyIdToGuid: Map<string, GUID>
componentPropertyDefinitionsById: ReadonlyMap<string, ComponentPropertyDefinition>
fractionalPosition: (index: number) => string
mapToFigmaType: (type: SceneNode['type']) => string
@ -148,8 +148,9 @@ function componentPropertyValue(
if (type === 'BOOLEAN') return { boolValue: value === 'true' }
if (type === 'INSTANCE_SWAP') {
const target = context.graph.getNode(value)
if (!target) return { textValue: { characters: value } }
const guid = getOrCreateNodeGuid(context, target.id, localIdCounter)
const guid = target
? getOrCreateNodeGuid(context, target.id, localIdCounter)
: parseGuidOrNull(value)
return guid ? { guidValue: guid } : { textValue: { characters: value } }
}
return { textValue: { characters: value } }
@ -372,10 +373,13 @@ function getOrCreatePropertyGuid(
propertyId: string,
localIdCounter: { value: number }
): GUID {
const existing = context.propertyIdToGuid?.get(propertyId)
const existing = context.propertyIdToGuid.get(propertyId)
if (existing) return existing
const guid = parseGuidOrNull(propertyId) ?? { sessionID: 1, localID: localIdCounter.value++ }
context.propertyIdToGuid?.set(propertyId, guid)
const parsed = parseGuidOrNull(propertyId)
if (parsed) return parsed
const guid = { sessionID: 1, localID: localIdCounter.value++ }
context.propertyIdToGuid.set(propertyId, guid)
context.assignedGuidValues?.add(`${guid.sessionID}:${guid.localID}`)
return guid
}
@ -624,7 +628,10 @@ function applyInstancePayload(
}
nc.symbolData = symbolData as KiwiNodeChange['symbolData']
}
if (node.source.fig.componentPropAssignments.length > 0) {
if (
node.source.fig.componentPropAssignments.length > 0 &&
!node.source.editedFields.includes('componentPropertyAssignments')
) {
nc.componentPropAssignments = materializeFigmaPayload(
node.source.fig.componentPropAssignments,
context.blobs,
@ -653,15 +660,14 @@ function applyInstancePayload(
function componentPropertyPreferredValues(
definition: ComponentPropertyDefinition,
context: SceneNodeToKiwiContext,
localIdCounter: { value: number }
context: SceneNodeToKiwiContext
) {
if (definition.type === 'INSTANCE_SWAP' && definition.preferredValues?.length) {
return {
instanceSwapValues: definition.preferredValues.map((nodeId) => {
const target = context.graph.getNode(nodeId)
const guid = target ? getOrCreateNodeGuid(context, target.id, localIdCounter) : undefined
return { type: 'COMPONENT', key: guid ? guidToString(guid) : nodeId }
instanceSwapValues: definition.preferredValues.map((value) => {
const target = context.graph.getNode(value)
const key = target?.componentKey || target?.sourceLibraryKey || value
return { type: 'COMPONENT', key }
})
}
}
@ -723,7 +729,7 @@ function applyComponentMetadata(
name: def.name,
type: componentPropertyTypeForKiwi(def.type),
initialValue: componentPropertyValue(def.type, def.defaultValue, context, localIdCounter),
preferredValues: componentPropertyPreferredValues(def, context, localIdCounter)
preferredValues: componentPropertyPreferredValues(def, context)
}))
if (shouldSerializeRawBackedField(node, 'componentPropDefs', componentPropDefs.length > 0)) {
nc.componentPropDefs = componentPropDefs

View file

@ -494,7 +494,7 @@ export function sceneNodeToKiwi(
runtime: FigNodeChangeExportRuntime = EMPTY_EXPORT_RUNTIME,
componentPropertyDefinitionsById = buildComponentPropIndex(graph),
modeIdToGuid?: Map<string, GUID>,
propertyIdToGuid?: Map<string, GUID>
propertyIdToGuid = new Map<string, GUID>()
): KiwiNodeChange[] {
// Raw paints retain library asset refs; effects use this map because their
// Kiwi schema accepts only GUID-backed aliases.

View file

@ -1,6 +1,7 @@
import { describe, expect, test } from 'bun:test'
import { SceneGraph } from '@open-pencil/scene-graph'
import type { GUID } from '@open-pencil/scene-graph/primitives'
import {
buildComponentPropIndex,
@ -151,7 +152,14 @@ describe('@open-pencil/fig SceneGraph export policy', () => {
]
})
const [change] = sceneNodeToKiwi(componentSet, { sessionID: 1, localID: 1 }, 0, { value: 2 }, graph, [])
const [change] = sceneNodeToKiwi(
componentSet,
{ sessionID: 1, localID: 1 },
0,
{ value: 2 },
graph,
[]
)
expect(change.componentPropDefs).toHaveLength(1)
expect(change.componentPropDefs?.[0].id).toEqual(
@ -172,8 +180,8 @@ describe('@open-pencil/fig SceneGraph export policy', () => {
componentPropertyReferences: [{ propertyId: 'prop:icon1234', field: 'INSTANCE_SWAP' }]
})
const nodeIdToGuid = new Map()
const propertyIdToGuid = new Map()
const nodeIdToGuid = new Map<string, GUID>()
const propertyIdToGuid = new Map<string, GUID>()
const localIdCounter = { value: 2 }
const [componentChange] = sceneNodeToKiwi(
component,
@ -212,21 +220,26 @@ describe('@open-pencil/fig SceneGraph export policy', () => {
propertyIdToGuid
)[0]
expect(componentChange.componentPropDefs?.[0].id).toEqual(slotChange.componentPropRefs?.[0].defID)
expect(componentChange.componentPropDefs?.[0].id).toEqual(
slotChange.componentPropRefs?.[0].defID
)
})
test('points an INSTANCE_SWAP default value at the same GUID the target component is exported with', () => {
const graph = new SceneGraph()
const page = graph.getPages()[0]
const icon = graph.createNode('COMPONENT', page.id, { name: 'Icon/Tune' })
const icon = graph.createNode('COMPONENT', page.id, {
name: 'Icon/Tune',
componentKey: 'icon-tune-key'
})
const button = graph.createNode('COMPONENT', page.id, {
componentPropertyDefinitions: [
{ id: 'prop:iconswap1', name: 'Icon', type: 'INSTANCE_SWAP', defaultValue: icon.id }
]
})
const nodeIdToGuid = new Map()
const propertyIdToGuid = new Map()
const nodeIdToGuid = new Map<string, GUID>()
const propertyIdToGuid = new Map<string, GUID>()
const localIdCounter = { value: 2 }
const [iconChange] = sceneNodeToKiwi(
icon,
@ -266,5 +279,87 @@ describe('@open-pencil/fig SceneGraph export policy', () => {
)
expect(buttonChange.componentPropDefs?.[0].initialValue).toEqual({ guidValue: iconChange.guid })
expect(buttonChange.componentPropDefs?.[0].preferredValues).toBeUndefined()
})
test('exports INSTANCE_SWAP preferred values as component keys', () => {
const graph = new SceneGraph()
const page = graph.getPages()[0]
const icon = graph.createNode('COMPONENT', page.id, {
name: 'Icon/Tune',
componentKey: 'icon-tune-key'
})
const button = graph.createNode('COMPONENT', page.id, {
componentPropertyDefinitions: [
{
id: 'prop:iconswap2',
name: 'Icon',
type: 'INSTANCE_SWAP',
defaultValue: icon.id,
preferredValues: [icon.id, 'external-library-key']
}
]
})
const [buttonChange] = sceneNodeToKiwi(
button,
{ sessionID: 1, localID: 1 },
0,
{ value: 2 },
graph,
[]
)
expect(buttonChange.componentPropDefs?.[0].preferredValues?.instanceSwapValues).toEqual([
{ type: 'COMPONENT', key: 'icon-tune-key' },
{ type: 'COMPONENT', key: 'external-library-key' }
])
})
test('preserves unresolved GUID-shaped INSTANCE_SWAP values as GUIDs', () => {
const graph = new SceneGraph()
const page = graph.getPages()[0]
const component = graph.createNode('COMPONENT', page.id, {
componentPropertyDefinitions: [
{ id: 'prop:iconswap3', name: 'Icon', type: 'INSTANCE_SWAP', defaultValue: '70:1' }
]
})
const [change] = sceneNodeToKiwi(
component,
{ sessionID: 1, localID: 1 },
0,
{ value: 2 },
graph,
[]
)
expect(change.componentPropDefs?.[0].initialValue).toEqual({
guidValue: { sessionID: 70, localID: 1 }
})
})
test('shares synthetic property GUIDs across recursive serialization without a supplied map', () => {
const graph = new SceneGraph()
const page = graph.getPages()[0]
const component = graph.createNode('COMPONENT', page.id, {
componentPropertyDefinitions: [
{ id: 'prop:recursive', name: 'Label', type: 'TEXT', defaultValue: 'Default' }
]
})
graph.createNode('TEXT', component.id, {
componentPropertyReferences: [{ propertyId: 'prop:recursive', field: 'TEXT' }]
})
const changes = sceneNodeToKiwi(
component,
{ sessionID: 1, localID: 1 },
0,
{ value: 2 },
graph,
[]
)
expect(changes[0].componentPropDefs?.[0].id).toEqual(changes[1].componentPropRefs?.[0].defID)
})
})

View file

@ -171,4 +171,47 @@ describe('Figma component property roundtrip', () => {
'30:2': 'false'
})
})
test('exports edited imported assignments instead of stale raw values', async () => {
const graph = new SceneGraph()
const page = graph.getPages()[0]
const component = graph.createNode('COMPONENT', page.id, {
name: 'Card',
componentPropertyDefinitions: [
{ id: '30:1', name: 'Label', type: 'TEXT', defaultValue: 'Default' }
]
})
component.source.id = '10:1'
const instance = graph.createInstance(component.id, page.id, {
name: 'Card instance',
componentPropertyAssignments: { '30:1': 'Original' }
})
if (!instance) throw new Error('Expected instance')
instance.source.id = '20:1'
const imported = importNodeChanges(
parseFigBuffer((await exportFigFile(graph)).buffer as ArrayBuffer).nodeChanges,
[],
undefined,
{ populate: 'all' }
)
const importedInstance = [...imported.getAllNodes()].find(
(node) => node.name === 'Card instance'
)
if (!importedInstance) throw new Error('Expected imported instance')
imported.updateNode(importedInstance.id, {
componentPropertyAssignments: { '30:1': 'Edited' }
})
const reloaded = importNodeChanges(
parseFigBuffer((await exportFigFile(imported)).buffer as ArrayBuffer).nodeChanges,
[],
undefined,
{ populate: 'all' }
)
const reloadedInstance = [...reloaded.getAllNodes()].find(
(node) => node.name === 'Card instance'
)
expect(reloadedInstance?.componentPropertyAssignments).toEqual({ '30:1': 'Edited' })
})
})

View file

@ -338,14 +338,20 @@ describe('Figma component property import', () => {
id: '3:2',
name: 'icon',
type: 'INSTANCE_SWAP',
defaultValue: '4:1',
defaultValue: expect.any(String),
preferredValues: ['icon/user-key']
}
])
const defaultTarget = component?.componentPropertyDefinitions[0]?.defaultValue
expect(defaultTarget ? graph.getNode(defaultTarget)?.name : undefined).toBe('icon/mail')
const sourceInstance = Array.from(graph.getAllNodes()).find(
(node) => node.name === 'Menu item source'
)
expect(sourceInstance?.componentPropertyAssignments).toEqual({ '3:2': '4:3' })
expect(sourceInstance?.componentPropertyAssignments).toEqual({
'3:2': expect.any(String)
})
const assignedTarget = sourceInstance?.componentPropertyAssignments['3:2']
expect(assignedTarget ? graph.getNode(assignedTarget)?.name : undefined).toBe('icon/user')
const clone = Array.from(graph.getAllNodes()).find((node) => node.name === 'Menu item clone')
const icon = clone?.childIds
.map((id) => graph.getNode(id))

View file

@ -10,8 +10,14 @@ describe('INSTANCE_SWAP component property round trip', () => {
const graph = new SceneGraph()
const page = graph.getPages()[0]
const iconA = graph.createNode('COMPONENT', page.id, { name: 'Icon/A' })
const iconB = graph.createNode('COMPONENT', page.id, { name: 'Icon/B' })
const iconA = graph.createNode('COMPONENT', page.id, {
name: 'Icon/A',
componentKey: 'icon-a-key'
})
const iconB = graph.createNode('COMPONENT', page.id, {
name: 'Icon/B',
componentKey: 'icon-b-key'
})
const button = graph.createNode('COMPONENT', page.id, {
name: 'Button',
componentPropertyDefinitions: [
@ -27,7 +33,8 @@ describe('INSTANCE_SWAP component property round trip', () => {
const slot = graph.createInstance(iconA.id, button.id)
if (!slot) throw new Error('failed to create slot instance')
graph.updateNode(slot.id, {
componentPropertyReferences: [{ propertyId: 'prop:iconswap01', field: 'INSTANCE_SWAP' }]
componentPropertyReferences: [{ propertyId: 'prop:iconswap01', field: 'INSTANCE_SWAP' }],
componentPropertyAssignments: { 'prop:iconswap01': iconB.id }
})
const exported = await exportFigFile(graph)
@ -44,12 +51,18 @@ describe('INSTANCE_SWAP component property round trip', () => {
const defaultTarget = def?.defaultValue ? reloaded.getNode(def.defaultValue) : undefined
expect(defaultTarget?.name).toBe('Icon/A')
const preferredTargets = (def?.preferredValues ?? []).map((id) => reloaded.getNode(id)?.name)
expect(preferredTargets.sort()).toEqual(['Icon/A', 'Icon/B'])
const preferredValues = new Set(def?.preferredValues)
expect(preferredValues).toEqual(new Set(['icon-a-key', 'icon-b-key']))
const reloadedSlot = reloadedButton ? reloaded.getChildren(reloadedButton.id)[0] : undefined
expect(reloadedSlot?.type).toBe('INSTANCE')
const slotComponent = reloadedSlot?.componentId ? reloaded.getNode(reloadedSlot.componentId) : undefined
const slotComponent = reloadedSlot?.componentId
? reloaded.getNode(reloadedSlot.componentId)
: undefined
expect(slotComponent?.name).toBe('Icon/A')
const assignment = reloadedSlot?.componentPropertyAssignments[def?.id ?? '']
const assignmentTarget = assignment ? reloaded.getNode(assignment) : undefined
expect(assignmentTarget?.name).toBe('Icon/B')
})
})