From 783cdf9dbc86fc6d60b5a348fc0863476451a0e4 Mon Sep 17 00:00:00 2001 From: xemc <0xemc@protonmail.com> Date: Tue, 18 Aug 2026 08:10:11 +1000 Subject: [PATCH 1/2] fix(fig): component property definitions/references/assignments never survive a save/reload round trip parseGuidOrNull requires the strict Figma GUID shape (sessionID:localID), but every OpenPencil code path that creates a component property definition uses `prop:` IDs, which never match. applyComponentMetadata silently dropped componentPropDefs/componentPropRefs/componentPropAssignments/ variantPropSpecs whenever any entry failed that check, so any component property created by the app (as opposed to imported straight from a real Figma file) vanished on the very next save. Mint a stable synthetic GUID for non-Figma-shaped property IDs instead of dropping them, memoized per export so defs/refs/assignments/variantPropSpecs pointing at the same property stay consistent. Also fix INSTANCE_SWAP default/preferred/assignment values, which reference target node IDs and had the same GUID-shape assumption, and were never being resolved back to real node IDs on import in the first place (remapComponentIds only handled instance.componentId, not these fields). Co-Authored-By: Claude Sonnet 5 --- packages/core/src/io/formats/fig/export.ts | 11 +- packages/core/src/kiwi/fig/import.ts | 55 +++++++- .../src/kiwi/fig/node-change/serialize.ts | 6 +- packages/fig/src/node-change/export-node.ts | 107 ++++++++------ packages/fig/src/node-change/serialize.ts | 4 +- packages/fig/tests/export.test.ts | 132 ++++++++++++++++++ .../io/fig/instance-swap-roundtrip.test.ts | 55 ++++++++ 7 files changed, 321 insertions(+), 49 deletions(-) create mode 100644 tests/engine/io/fig/instance-swap-roundtrip.test.ts diff --git a/packages/core/src/io/formats/fig/export.ts b/packages/core/src/io/formats/fig/export.ts index ca5d0b6cd..c9e057dd7 100644 --- a/packages/core/src/io/formats/fig/export.ts +++ b/packages/core/src/io/formats/fig/export.ts @@ -343,6 +343,7 @@ interface InternalResourceContext { blobIndexByHex: Map assignedGuidValues: Set componentPropertyDefinitionsById: ReturnType + propertyIdToGuid: Map } function appendInternalResources(context: InternalResourceContext): void { @@ -365,7 +366,8 @@ function appendInternalResources(context: InternalResourceContext): void { context.blobIndexByHex, context.assignedGuidValues, context.componentPropertyDefinitionsById, - context.modeIdToGuid + context.modeIdToGuid, + context.propertyIdToGuid ) ) } @@ -429,6 +431,7 @@ export async function exportFigFile( assignedGuidValues.add(`${docGuid.sessionID}:${docGuid.localID}`) const varIdToGuid = new Map() const modeIdToGuid = new Map() + const propertyIdToGuid = new Map() const fontDigestMap = await buildFontDigestMap(graph) const glyphBlobMap = new Map() const blobIndexByHex = new Map() @@ -499,7 +502,8 @@ export async function exportFigFile( blobIndexByHex, assignedGuidValues, componentPropertyDefinitionsById, - modeIdToGuid + modeIdToGuid, + propertyIdToGuid ) ) } @@ -518,7 +522,8 @@ export async function exportFigFile( glyphBlobMap, blobIndexByHex, assignedGuidValues, - componentPropertyDefinitionsById + componentPropertyDefinitionsById, + propertyIdToGuid }) const msg: Record = { diff --git a/packages/core/src/kiwi/fig/import.ts b/packages/core/src/kiwi/fig/import.ts index 5ae529b39..bec4b5984 100644 --- a/packages/core/src/kiwi/fig/import.ts +++ b/packages/core/src/kiwi/fig/import.ts @@ -15,7 +15,11 @@ import { } from '@open-pencil/fig/node-change' import type { NodeChange, VariableDataValuesEntry, Color, GUID } from '@open-pencil/kiwi/fig/codec' import { SceneGraph } from '@open-pencil/scene-graph' -import type { VariableType, VariableValue } from '@open-pencil/scene-graph' +import type { + ComponentPropertyDefinition, + VariableType, + VariableValue +} from '@open-pencil/scene-graph' import { BLACK } from '#core/constants' import { setLazyFigImportContext } from '#core/kiwi/fig/lazy-import' @@ -394,6 +398,54 @@ function remapComponentIds(graph: SceneGraph, guidToNodeId: Map) }) } +/** + * INSTANCE_SWAP definitions/assignments store a target node's GUID (matching + * 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): void { + const defsById = new Map() + for (const node of graph.getAllNodes()) { + for (const def of node.componentPropertyDefinitions) { + if (!defsById.has(def.id)) defsById.set(def.id, def) + } + } + + graph.preserveSourceMetadataDuring(() => { + for (const node of graph.getAllNodes()) { + if (node.componentPropertyDefinitions.length > 0) { + 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 + } + }) + const changed = defs.some((def, i) => def !== node.componentPropertyDefinitions[i]) + if (changed) graph.updateNode(node.id, { componentPropertyDefinitions: defs }) + } + + if (Object.keys(node.componentPropertyAssignments).length > 0) { + let changed = false + const assignments = { ...node.componentPropertyAssignments } + for (const [propId, value] of Object.entries(assignments)) { + if (defsById.get(propId)?.type !== 'INSTANCE_SWAP') continue + const remapped = guidToNodeId.get(value) + if (remapped) { + assignments[propId] = remapped + changed = true + } + } + if (changed) graph.updateNode(node.id, { componentPropertyAssignments: assignments }) + } + } + }) +} + function applyVariantPropSpecs(graph: SceneGraph): void { for (const node of graph.getAllNodes()) { if (node.type !== 'COMPONENT' || node.variantPropSpecs.length === 0 || !node.parentId) continue @@ -509,6 +561,7 @@ export function importNodeChanges( importVariableEntries(changeMap, parentMap, graph, assetRefs) importVariableBindings(changeMap, guidToNodeId, graph) remapComponentIds(graph, guidToNodeId) + remapInstanceSwapPropertyValues(graph, guidToNodeId) applyVariantPropSpecs(graph) const firstPageId = graph.getPages()[0]?.id diff --git a/packages/core/src/kiwi/fig/node-change/serialize.ts b/packages/core/src/kiwi/fig/node-change/serialize.ts index 6e97ee40b..9cf8b3ad3 100644 --- a/packages/core/src/kiwi/fig/node-change/serialize.ts +++ b/packages/core/src/kiwi/fig/node-change/serialize.ts @@ -38,7 +38,8 @@ export function sceneNodeToKiwi( blobIndexByHex?: Map, assignedGuidValues?: Set, componentPropertyDefinitionsById?: ReadonlyMap, - modeIdToGuid?: Map + modeIdToGuid?: Map, + propertyIdToGuid?: Map ): KiwiNodeChange[] { return sceneNodeToKiwiWithRuntime( node, @@ -55,6 +56,7 @@ export function sceneNodeToKiwi( assignedGuidValues, coreFigExportRuntime, componentPropertyDefinitionsById, - modeIdToGuid + modeIdToGuid, + propertyIdToGuid ) } diff --git a/packages/fig/src/node-change/export-node.ts b/packages/fig/src/node-change/export-node.ts index 1596b9547..c207e8ae3 100644 --- a/packages/fig/src/node-change/export-node.ts +++ b/packages/fig/src/node-change/export-node.ts @@ -1,5 +1,5 @@ import type { NodeChange, Paint } from '@open-pencil/kiwi/fig/codec' -import { stringToGuid } from '@open-pencil/kiwi/fig/guid' +import { guidToString, stringToGuid } from '@open-pencil/kiwi/fig/guid' import { DEFAULT_STROKE_MITER_LIMIT } from '@open-pencil/scene-graph' import type { ComponentPropertyDefinition, @@ -65,6 +65,10 @@ interface SceneNodeToKiwiContext { modeIdToGuid?: Map /** Variable GUIDs used only where raw effect aliases cannot retain asset refs. */ assetRefToVarGuid?: Map + /** 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 componentPropertyDefinitionsById: ReadonlyMap fractionalPosition: (index: number) => string mapToFigmaType: (type: SceneNode['type']) => string @@ -135,11 +139,17 @@ function componentPropertyTypeForKiwi(type: string) { return type } -function componentPropertyValue(type: string, value: string, graph: SceneGraph) { +function componentPropertyValue( + type: string, + value: string, + context: SceneNodeToKiwiContext, + localIdCounter: { value: number } +) { if (type === 'BOOLEAN') return { boolValue: value === 'true' } if (type === 'INSTANCE_SWAP') { - const target = graph.getNode(value) - const guid = parseGuidOrNull(target?.source.id ?? value) + const target = context.graph.getNode(value) + if (!target) return { textValue: { characters: value } } + const guid = getOrCreateNodeGuid(context, target.id, localIdCounter) return guid ? { guidValue: guid } : { textValue: { characters: value } } } return { textValue: { characters: value } } @@ -351,6 +361,24 @@ function getOrCreateNodeGuid( return guid } +/** + * Component property IDs ("prop:abc123") never match the Figma GUID shape, + * so parseGuidOrNull always rejects them — mint a stable synthetic GUID from + * the shared node-id counter instead, memoized so every def/ref/assignment/ + * variantPropSpec pointing at the same property ID round-trips consistently. + */ +function getOrCreatePropertyGuid( + context: SceneNodeToKiwiContext, + propertyId: string, + localIdCounter: { value: number } +): GUID { + const existing = context.propertyIdToGuid?.get(propertyId) + if (existing) return existing + const guid = parseGuidOrNull(propertyId) ?? { sessionID: 1, localID: localIdCounter.value++ } + context.propertyIdToGuid?.set(propertyId, guid) + return guid +} + function isDescendantOf(context: SceneNodeToKiwiContext, nodeId: string, ancestorId: string) { let current = context.graph.getNode(nodeId) while (current?.parentId) { @@ -623,10 +651,18 @@ function applyInstancePayload( } } -function componentPropertyPreferredValues(definition: ComponentPropertyDefinition) { +function componentPropertyPreferredValues( + definition: ComponentPropertyDefinition, + context: SceneNodeToKiwiContext, + localIdCounter: { value: number } +) { if (definition.type === 'INSTANCE_SWAP' && definition.preferredValues?.length) { return { - instanceSwapValues: definition.preferredValues.map((key) => ({ type: 'COMPONENT', key })) + 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 } + }) } } if (definition.type === 'VARIANT' && definition.variantOptions?.length) { @@ -665,7 +701,8 @@ function shouldSerializeRawBackedField( function applyComponentMetadata( context: SceneNodeToKiwiContext, node: SceneNode, - nc: KiwiNodeChange + nc: KiwiNodeChange, + localIdCounter: { value: number } ): void { if (node.componentKey) nc.componentKey = node.componentKey if (node.sourceLibraryKey) nc.sourceLibraryKey = node.sourceLibraryKey @@ -681,45 +718,33 @@ function applyComponentMetadata( } if (node.symbolDescription) nc.symbolDescription = node.symbolDescription if (node.symbolLinks.length > 0) nc.symbolLinks = structuredClone(node.symbolLinks) - const componentPropDefs = node.componentPropertyDefinitions - .map((def) => { - const id = parseGuidOrNull(def.id) - return id - ? { - id, - name: def.name, - type: componentPropertyTypeForKiwi(def.type), - initialValue: componentPropertyValue(def.type, def.defaultValue, context.graph), - preferredValues: componentPropertyPreferredValues(def) - } - : null - }) - .filter((def): def is NonNullable => def !== null) + const componentPropDefs = node.componentPropertyDefinitions.map((def) => ({ + id: getOrCreatePropertyGuid(context, def.id, localIdCounter), + name: def.name, + type: componentPropertyTypeForKiwi(def.type), + initialValue: componentPropertyValue(def.type, def.defaultValue, context, localIdCounter), + preferredValues: componentPropertyPreferredValues(def, context, localIdCounter) + })) if (shouldSerializeRawBackedField(node, 'componentPropDefs', componentPropDefs.length > 0)) { nc.componentPropDefs = componentPropDefs } - const componentPropRefs = node.componentPropertyReferences - .map((ref) => { - const defID = parseGuidOrNull(ref.propertyId) - if (!defID) return null - return { defID, componentPropNodeField: componentPropertyNodeField(ref.field) } - }) - .filter((ref): ref is NonNullable => ref !== null) + const componentPropRefs = node.componentPropertyReferences.map((ref) => ({ + defID: getOrCreatePropertyGuid(context, ref.propertyId, localIdCounter), + componentPropNodeField: componentPropertyNodeField(ref.field) + })) if (shouldSerializeRawBackedField(node, 'componentPropRefs', componentPropRefs.length > 0)) { nc.componentPropRefs = componentPropRefs } const componentPropAssignments = Object.entries(node.componentPropertyAssignments) .map(([propertyId, value]) => { - const defID = parseGuidOrNull(propertyId) const definition = context.componentPropertyDefinitionsById.get(propertyId) - return defID && definition - ? { - defID, - value: componentPropertyValue(definition.type, value, context.graph) - } - : null + if (!definition) return null + return { + defID: getOrCreatePropertyGuid(context, propertyId, localIdCounter), + value: componentPropertyValue(definition.type, value, context, localIdCounter) + } }) .filter((assignment): assignment is NonNullable => assignment !== null) if ( @@ -733,12 +758,10 @@ function applyComponentMetadata( nc.componentPropAssignments = componentPropAssignments } - const variantPropSpecs = node.variantPropSpecs - .map((spec) => { - const propDefId = parseGuidOrNull(spec.propDefId) - return propDefId ? { propDefId, value: spec.value } : null - }) - .filter((spec): spec is NonNullable => spec !== null) + const variantPropSpecs = node.variantPropSpecs.map((spec) => ({ + propDefId: getOrCreatePropertyGuid(context, spec.propDefId, localIdCounter), + value: spec.value + })) if (shouldSerializeRawBackedField(node, 'variantPropSpecs', variantPropSpecs.length > 0)) { nc.variantPropSpecs = variantPropSpecs } @@ -941,7 +964,7 @@ export function sceneNodeToKiwiWithContext( if (node.locked) nc.locked = true applyNodeVisualProps(context, node, nc) - applyComponentMetadata(context, node, nc) + applyComponentMetadata(context, node, nc, localIdCounter) applyInstancePayload(context, node, nc, localIdCounter) if (node.type === 'COMPONENT_SET') upsertPluginData(node, NODE_TYPE_PLUGIN_KEY, node.type) if (nc.type === 'CANVAS') nc.pageType = 'DESIGN' diff --git a/packages/fig/src/node-change/serialize.ts b/packages/fig/src/node-change/serialize.ts index 084933a2f..a3cc4a2fe 100644 --- a/packages/fig/src/node-change/serialize.ts +++ b/packages/fig/src/node-change/serialize.ts @@ -493,7 +493,8 @@ export function sceneNodeToKiwi( assignedGuidValues?: Set, runtime: FigNodeChangeExportRuntime = EMPTY_EXPORT_RUNTIME, componentPropertyDefinitionsById = buildComponentPropIndex(graph), - modeIdToGuid?: Map + modeIdToGuid?: Map, + propertyIdToGuid?: Map ): KiwiNodeChange[] { // Raw paints retain library asset refs; effects use this map because their // Kiwi schema accepts only GUID-backed aliases. @@ -510,6 +511,7 @@ export function sceneNodeToKiwi( modeIdToGuid, assetRefToVarGuid, componentPropertyDefinitionsById, + propertyIdToGuid, fractionalPosition, mapToFigmaType, fillToKiwiPaint, diff --git a/packages/fig/tests/export.test.ts b/packages/fig/tests/export.test.ts index 925672431..9a1056cf0 100644 --- a/packages/fig/tests/export.test.ts +++ b/packages/fig/tests/export.test.ts @@ -135,4 +135,136 @@ describe('@open-pencil/fig SceneGraph export policy', () => { expect(change.derivedTextData?.glyphs).toHaveLength(1) expect(blobs).toHaveLength(1) }) + + test('mints a synthetic GUID for app-created (non-Figma-shaped) component property IDs', () => { + const graph = new SceneGraph() + const page = graph.getPages()[0] + const componentSet = graph.createNode('COMPONENT_SET', page.id, { + componentPropertyDefinitions: [ + { + id: 'prop:abc12345', + name: 'Style', + type: 'VARIANT', + defaultValue: 'Primary', + variantOptions: ['Primary', 'Secondary'] + } + ] + }) + + const [change] = sceneNodeToKiwi(componentSet, { sessionID: 1, localID: 1 }, 0, { value: 2 }, graph, []) + + expect(change.componentPropDefs).toHaveLength(1) + expect(change.componentPropDefs?.[0].id).toEqual( + expect.objectContaining({ sessionID: expect.any(Number), localID: expect.any(Number) }) + ) + expect(change.componentPropDefs?.[0].name).toBe('Style') + }) + + test('reuses the same synthetic GUID for a def and the ref that points at it', () => { + const graph = new SceneGraph() + const page = graph.getPages()[0] + const component = graph.createNode('COMPONENT', page.id, { + componentPropertyDefinitions: [ + { id: 'prop:icon1234', name: 'Icon', type: 'INSTANCE_SWAP', defaultValue: '' } + ] + }) + const slot = graph.createNode('INSTANCE', component.id, { + componentPropertyReferences: [{ propertyId: 'prop:icon1234', field: 'INSTANCE_SWAP' }] + }) + + const nodeIdToGuid = new Map() + const propertyIdToGuid = new Map() + const localIdCounter = { value: 2 } + const [componentChange] = sceneNodeToKiwi( + component, + { sessionID: 1, localID: 1 }, + 0, + localIdCounter, + graph, + [], + nodeIdToGuid, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + propertyIdToGuid + ) + const slotChange = sceneNodeToKiwi( + slot, + componentChange.guid, + 0, + localIdCounter, + graph, + [], + nodeIdToGuid, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + propertyIdToGuid + )[0] + + 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 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 localIdCounter = { value: 2 } + const [iconChange] = sceneNodeToKiwi( + icon, + { sessionID: 1, localID: 1 }, + 0, + localIdCounter, + graph, + [], + nodeIdToGuid, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + propertyIdToGuid + ) + const [buttonChange] = sceneNodeToKiwi( + button, + { sessionID: 1, localID: 1 }, + 1, + localIdCounter, + graph, + [], + nodeIdToGuid, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + propertyIdToGuid + ) + + expect(buttonChange.componentPropDefs?.[0].initialValue).toEqual({ guidValue: iconChange.guid }) + }) }) diff --git a/tests/engine/io/fig/instance-swap-roundtrip.test.ts b/tests/engine/io/fig/instance-swap-roundtrip.test.ts new file mode 100644 index 000000000..82389a483 --- /dev/null +++ b/tests/engine/io/fig/instance-swap-roundtrip.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, test } from 'bun:test' + +import { exportFigFile, parseFigFile } from '@open-pencil/core/io' +import { initCodec } from '@open-pencil/core/kiwi' +import { SceneGraph } from '@open-pencil/scene-graph' + +describe('INSTANCE_SWAP component property round trip', () => { + test('defaultValue, preferredValues, and componentId survive a save/reload cycle', async () => { + await initCodec() + + 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 button = graph.createNode('COMPONENT', page.id, { + name: 'Button', + componentPropertyDefinitions: [ + { + id: 'prop:iconswap01', + name: 'Icon', + type: 'INSTANCE_SWAP', + defaultValue: iconA.id, + preferredValues: [iconA.id, iconB.id] + } + ] + }) + 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' }] + }) + + const exported = await exportFigFile(graph) + const reloaded = await parseFigFile(exported.buffer as ArrayBuffer) + + const reloadedButton = reloaded + .getAllNodes() + .find((n) => n.type === 'COMPONENT' && n.name === 'Button') + expect(reloadedButton).toBeDefined() + + const def = reloadedButton?.componentPropertyDefinitions[0] + expect(def?.type).toBe('INSTANCE_SWAP') + + 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 reloadedSlot = reloadedButton ? reloaded.getChildren(reloadedButton.id)[0] : undefined + expect(reloadedSlot?.type).toBe('INSTANCE') + const slotComponent = reloadedSlot?.componentId ? reloaded.getNode(reloadedSlot.componentId) : undefined + expect(slotComponent?.name).toBe('Icon/A') + }) +}) From 33962b764c05a09eda6e57c338555116a4eab72f Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Wed, 19 Aug 2026 18:05:25 +0300 Subject: [PATCH 2/2] 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 --- CHANGELOG.md | 1 + packages/core/src/io/formats/fig/export.ts | 56 +++++++++ packages/core/src/kiwi/fig/import.ts | 14 +-- packages/fig/src/node-change/export-node.ts | 36 +++--- packages/fig/src/node-change/serialize.ts | 2 +- packages/fig/tests/export.test.ts | 109 ++++++++++++++++-- .../fig/export/component-properties.test.ts | 43 +++++++ .../io/fig/import/component-props.test.ts | 10 +- .../io/fig/instance-swap-roundtrip.test.ts | 25 +++- 9 files changed, 257 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df1a3bd32..c7f970025 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/packages/core/src/io/formats/fig/export.ts b/packages/core/src/io/formats/fig/export.ts index 08497f764..267563456 100644 --- a/packages/core/src/io/formats/fig/export.ts +++ b/packages/core/src/io/formats/fig/export.ts @@ -163,6 +163,51 @@ function assignVariableGuids( } } +interface ComponentPropertyGuidState { + ids: string[] + maxLocalId0: number + maxLocalId1: number +} + +function collectComponentPropertyGuidState(graph: SceneGraph): ComponentPropertyGuidState { + const ids = new Set() + 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, + assignedGuidValues: Set, + nodeSourceGuidValues: Set +): 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 = [ diff --git a/packages/core/src/kiwi/fig/import.ts b/packages/core/src/kiwi/fig/import.ts index 1d3648985..8ba2a9ba2 100644 --- a/packages/core/src/kiwi/fig/import.ts +++ b/packages/core/src/kiwi/fig/import.ts @@ -407,7 +407,10 @@ function remapComponentIds(graph: SceneGraph, guidToNodeId: Map) * 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): void { +function remapInstanceSwapPropertyValues( + graph: SceneGraph, + guidToNodeId: Map +): void { const defsById = new Map() for (const node of graph.getAllNodes()) { for (const def of node.componentPropertyDefinitions) { @@ -421,13 +424,8 @@ function remapInstanceSwapPropertyValues(graph: SceneGraph, guidToNodeId: Map { 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 }) diff --git a/packages/fig/src/node-change/export-node.ts b/packages/fig/src/node-change/export-node.ts index c207e8ae3..f1b1fa3c6 100644 --- a/packages/fig/src/node-change/export-node.ts +++ b/packages/fig/src/node-change/export-node.ts @@ -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 + propertyIdToGuid: Map componentPropertyDefinitionsById: ReadonlyMap 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 diff --git a/packages/fig/src/node-change/serialize.ts b/packages/fig/src/node-change/serialize.ts index a3cc4a2fe..c8c25f754 100644 --- a/packages/fig/src/node-change/serialize.ts +++ b/packages/fig/src/node-change/serialize.ts @@ -494,7 +494,7 @@ export function sceneNodeToKiwi( runtime: FigNodeChangeExportRuntime = EMPTY_EXPORT_RUNTIME, componentPropertyDefinitionsById = buildComponentPropIndex(graph), modeIdToGuid?: Map, - propertyIdToGuid?: Map + propertyIdToGuid = new Map() ): KiwiNodeChange[] { // Raw paints retain library asset refs; effects use this map because their // Kiwi schema accepts only GUID-backed aliases. diff --git a/packages/fig/tests/export.test.ts b/packages/fig/tests/export.test.ts index 9a1056cf0..760b84c98 100644 --- a/packages/fig/tests/export.test.ts +++ b/packages/fig/tests/export.test.ts @@ -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() + const propertyIdToGuid = new Map() 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() + const propertyIdToGuid = new Map() 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) }) }) diff --git a/tests/engine/io/fig/export/component-properties.test.ts b/tests/engine/io/fig/export/component-properties.test.ts index 35321f4b8..a830732d2 100644 --- a/tests/engine/io/fig/export/component-properties.test.ts +++ b/tests/engine/io/fig/export/component-properties.test.ts @@ -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' }) + }) }) diff --git a/tests/engine/io/fig/import/component-props.test.ts b/tests/engine/io/fig/import/component-props.test.ts index 7069132e3..0925ceef4 100644 --- a/tests/engine/io/fig/import/component-props.test.ts +++ b/tests/engine/io/fig/import/component-props.test.ts @@ -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)) diff --git a/tests/engine/io/fig/instance-swap-roundtrip.test.ts b/tests/engine/io/fig/instance-swap-roundtrip.test.ts index 82389a483..d18d2d4fd 100644 --- a/tests/engine/io/fig/instance-swap-roundtrip.test.ts +++ b/tests/engine/io/fig/instance-swap-roundtrip.test.ts @@ -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') }) })