SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(fig): component property definitions/references/assignments never survive a save/reload round trip

Browse source 
parseGuidOrNull requires the strict Figma GUID shape (sessionID:localID),
but every OpenPencil code path that creates a component property definition
uses `prop:<hex>` 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 <noreply@anthropic.com>
This commit is contained in:
xemc 2026-08-18 08:10:11 +10:00
parent 15bd0ba19f
commit 783cdf9dbc
7 changed files with 321 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
packages/core/src/io/formats/fig/export.ts
View file

@ -343,6 +343,7 @@ interface InternalResourceContext {
blobIndexByHex: Map<string, number>
assignedGuidValues: Set<string>
componentPropertyDefinitionsById: ReturnType<typeof buildComponentPropIndex>
propertyIdToGuid: Map<string, GUID>
}
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<string, GUID>()
const modeIdToGuid = new Map<string, GUID>()
const propertyIdToGuid = new Map<string, GUID>()
const fontDigestMap = await buildFontDigestMap(graph)
const glyphBlobMap = new Map<string, number>()
const blobIndexByHex = new Map<string, number>()
@ -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<string, unknown> = {

55
packages/core/src/kiwi/fig/import.ts
View file

@ -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<string, string>)
})
}
/**
* 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<string, string>): void {
const defsById = new Map<string, ComponentPropertyDefinition>()
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

6
packages/core/src/kiwi/fig/node-change/serialize.ts
View file

@ -38,7 +38,8 @@ export function sceneNodeToKiwi(
blobIndexByHex?: Map<string, number>,
assignedGuidValues?: Set<string>,
componentPropertyDefinitionsById?: ReadonlyMap<string, ComponentPropertyDefinition>,
modeIdToGuid?: Map<string, GUID>
modeIdToGuid?: Map<string, GUID>,
propertyIdToGuid?: Map<string, GUID>
): KiwiNodeChange[] {
return sceneNodeToKiwiWithRuntime(
node,
@ -55,6 +56,7 @@ export function sceneNodeToKiwi(
assignedGuidValues,
coreFigExportRuntime,
componentPropertyDefinitionsById,
modeIdToGuid
modeIdToGuid,
propertyIdToGuid
)
}

107
packages/fig/src/node-change/export-node.ts
View file

@ -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<string, GUID>
/** Variable GUIDs used only where raw effect aliases cannot retain asset refs. */
assetRefToVarGuid?: Map<string, GUID>
/** 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>
componentPropertyDefinitionsById: ReadonlyMap<string, ComponentPropertyDefinition>
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<typeof def> => 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<typeof ref> => 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<typeof assignment> => 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<typeof spec> => 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'

4
packages/fig/src/node-change/serialize.ts
View file

@ -493,7 +493,8 @@ export function sceneNodeToKiwi(
assignedGuidValues?: Set<string>,
runtime: FigNodeChangeExportRuntime = EMPTY_EXPORT_RUNTIME,
componentPropertyDefinitionsById = buildComponentPropIndex(graph),
modeIdToGuid?: Map<string, GUID>
modeIdToGuid?: Map<string, GUID>,
propertyIdToGuid?: Map<string, GUID>
): 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,

132
packages/fig/tests/export.test.ts
View file

@ -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 })
})
})

55
tests/engine/io/fig/instance-swap-roundtrip.test.ts Normal file
View file

@ -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')
})
})