From 96de4e5fdbd9cc5ca018de1664ced564e230c758 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Tue, 21 Jul 2026 01:51:27 +0300 Subject: [PATCH] fix(fig): preserve regenerated override lineage - Remap descendant clone sources when instance branches are repopulated - Keep same-name component-property siblings distinct - Name swapped variants after their component sets --- .../component-props/assignments.ts | 8 +- packages/fig/src/instance-overrides/index.ts | 6 +- .../fig/src/instance-overrides/resolve.ts | 20 ++- .../fig/src/instance-overrides/sync/clones.ts | 31 +++- .../src/instance-overrides/sync/propagate.ts | 6 +- .../src/instance-overrides/sync/sources.ts | 132 ++++++++++++++++++ .../io/fig/import/component-props.test.ts | 67 +++++++++ tests/engine/io/fig/import/edge-cases.test.ts | 69 +++++++++ .../instance-overrides/sync-clones.test.ts | 10 +- 9 files changed, 337 insertions(+), 12 deletions(-) create mode 100644 packages/fig/src/instance-overrides/sync/sources.ts diff --git a/packages/fig/src/instance-overrides/component-props/assignments.ts b/packages/fig/src/instance-overrides/component-props/assignments.ts index 3297ccf15..177754cdb 100644 --- a/packages/fig/src/instance-overrides/component-props/assignments.ts +++ b/packages/fig/src/instance-overrides/component-props/assignments.ts @@ -40,9 +40,11 @@ function sourceChildPropRefs( for (const sourceChildId of sourceParent.childIds) { const sourceChild = ctx.graph.getNode(sourceChildId) if (!sourceChild) continue - if (sourceChild.componentId && sourceChild.componentId === child.componentId) { - const refs = findPropRefs(ctx, sourceChild.id, propRefsMap) - if (refs) return refs + if ( + sourceChild.id === child.componentId || + (sourceChild.componentId && sourceChild.componentId === child.componentId) + ) { + return findPropRefs(ctx, sourceChild.id, propRefsMap) ?? [] } if (!fallbackMatchId && sourceChild.name === child.name && sourceChild.type === child.type) { fallbackMatchId = sourceChild.id diff --git a/packages/fig/src/instance-overrides/index.ts b/packages/fig/src/instance-overrides/index.ts index b953af000..0d7516eff 100644 --- a/packages/fig/src/instance-overrides/index.ts +++ b/packages/fig/src/instance-overrides/index.ts @@ -33,6 +33,7 @@ import { populateInstances } from './populate' import { preComputeRoots } from './resolve' import { applySymbolOverrides } from './symbol/overrides' import { propagateNodePropsTransitively, propagateOverridesTransitively } from './sync' +import { indexCloneNodes } from './sync/sources' import type { InstanceNodeChange, OverrideContext, ComponentPropValue } from './types' /** @@ -302,7 +303,10 @@ export function populateAndApplyOverrides( if (activeRootIds) { const populated = populateInstances(graph, activeRootIds) - if (populated) ctx.activeNodeIds = populated + if (populated) { + ctx.activeNodeIds = populated + indexCloneNodes(graph, populated, ctx.preComputedClones) + } const latePropModified = applyComponentProperties(ctx) const lateSeeds = new Set([...overriddenNodes, ...propModified, ...latePropModified]) if (lateSeeds.size > 0) { diff --git a/packages/fig/src/instance-overrides/resolve.ts b/packages/fig/src/instance-overrides/resolve.ts index f04f4dd42..17acc9593 100644 --- a/packages/fig/src/instance-overrides/resolve.ts +++ b/packages/fig/src/instance-overrides/resolve.ts @@ -3,6 +3,11 @@ import type { GUID } from '@open-pencil/kiwi/fig/codec' import type { SceneNode } from '@open-pencil/scene-graph' import { copyStrokes } from '@open-pencil/scene-graph/copy' +import { + indexCloneSubtree, + remapRepopulatedChildSources, + snapshotChildSources +} from './sync/sources' import type { InstanceNodeChange, OverrideContext } from './types' const MAX_CHAIN_DEPTH = 20 @@ -437,24 +442,35 @@ function applyStrokeDescendants( visit(nodeId) } +function componentInstanceName(ctx: OverrideContext, component: SceneNode | undefined): string { + if (!component) return '' + const parent = component.parentId ? ctx.graph.getNode(component.parentId) : undefined + return parent?.type === 'COMPONENT_SET' ? parent.name : component.name +} + export function repopulateInstance(ctx: OverrideContext, nodeId: string, compId: string): void { const node = ctx.graph.getNode(nodeId) if (node?.type !== 'INSTANCE') return const previousStrokes = collectStyledStrokeDescendants(ctx, nodeId) + const previousSources = snapshotChildSources(ctx.graph, nodeId) const rootCompId = node.componentId ? getComponentRoot(ctx, node.componentId) : undefined const rootComp = rootCompId ? ctx.graph.getNode(rootCompId) : undefined for (const childId of Array.from(node.childIds)) ctx.graph.deleteNode(childId) const comp = ctx.graph.getNode(compId) const updates: Partial = { componentId: compId } - if (comp?.name && rootComp?.name && node.name === rootComp.name) { - updates.name = comp.name + const previousName = componentInstanceName(ctx, rootComp) + const nextName = componentInstanceName(ctx, comp) + if (nextName && previousName && (node.name === previousName || node.name === rootComp?.name)) { + updates.name = nextName } ctx.graph.preserveSourceMetadataDuring(() => ctx.graph.updateNode(nodeId, updates)) if (comp && comp.childIds.length > 0) { ctx.graph.populateInstanceChildren(nodeId, compId, 'fig-import') + indexCloneSubtree(ctx.graph, nodeId, ctx.preComputedClones) applyStrokeDescendants(ctx, nodeId, previousStrokes) } + remapRepopulatedChildSources(ctx.graph, nodeId, previousSources, ctx.preComputedClones) ctx.swappedInstances.add(nodeId) ctx.componentIdRoot.clear() candidateCache.delete(ctx) diff --git a/packages/fig/src/instance-overrides/sync/clones.ts b/packages/fig/src/instance-overrides/sync/clones.ts index 193c1a897..c009d8051 100644 --- a/packages/fig/src/instance-overrides/sync/clones.ts +++ b/packages/fig/src/instance-overrides/sync/clones.ts @@ -2,23 +2,29 @@ import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph' import type { ProtectionMap } from '../patches' import { syncNodeProps } from './fields' +import { indexCloneSubtree, remapRepopulatedChildSources, snapshotChildSources } from './sources' export function recloneChildren( graph: SceneGraph, srcChildId: string, tgtNode: SceneNode, swappedInstances: Set, - protections?: ProtectionMap + protections?: ProtectionMap, + cloneSources?: Map ): void { const srcChild = graph.getNode(srcChildId) if (!srcChild) return + const effectiveCloneSources = cloneSources ?? buildClonesMap(graph) + const previousSources = snapshotChildSources(graph, tgtNode.id) for (const childId of Array.from(tgtNode.childIds)) graph.deleteNode(childId) graph.updateNode(tgtNode.id, { name: srcChild.name, componentId: srcChild.componentId }) syncNodeProps(graph, srcChild, tgtNode, protections) if (srcChild.childIds.length > 0) { graph.populateInstanceChildren(tgtNode.id, srcChildId, 'fig-import') + indexCloneSubtree(graph, tgtNode.id, effectiveCloneSources) } + remapRepopulatedChildSources(graph, tgtNode.id, previousSources, effectiveCloneSources) swappedInstances.add(tgtNode.id) } @@ -28,11 +34,13 @@ export function syncChildrenDeep( targetId: string, swappedInstances: Set, skip?: Set, - protections?: ProtectionMap + protections?: ProtectionMap, + cloneSources?: Map ): void { const src = graph.getNode(sourceId) const tgt = graph.getNode(targetId) if (!src || !tgt) return + const effectiveCloneSources = cloneSources ?? buildClonesMap(graph) const len = Math.min(src.childIds.length, tgt.childIds.length) for (let i = 0; i < len; i++) { if (skip?.has(tgt.childIds[i])) continue @@ -41,12 +49,27 @@ export function syncChildrenDeep( if (!srcNode || !tgtNode || srcNode.type !== tgtNode.type) continue if (srcNode.type === 'INSTANCE' && srcNode.componentId !== tgtNode.componentId) { - recloneChildren(graph, src.childIds[i], tgtNode, swappedInstances, protections) + recloneChildren( + graph, + src.childIds[i], + tgtNode, + swappedInstances, + protections, + effectiveCloneSources + ) continue } syncNodeProps(graph, srcNode, tgtNode, protections) - syncChildrenDeep(graph, src.childIds[i], tgt.childIds[i], swappedInstances, skip, protections) + syncChildrenDeep( + graph, + src.childIds[i], + tgt.childIds[i], + swappedInstances, + skip, + protections, + effectiveCloneSources + ) } } diff --git a/packages/fig/src/instance-overrides/sync/propagate.ts b/packages/fig/src/instance-overrides/sync/propagate.ts index a50cbad8a..2d988e818 100644 --- a/packages/fig/src/instance-overrides/sync/propagate.ts +++ b/packages/fig/src/instance-overrides/sync/propagate.ts @@ -3,6 +3,7 @@ import type { SceneGraph } from '@open-pencil/scene-graph' import type { ProtectionMap } from '../patches' import { buildClonesMap, syncChildrenDeep } from './clones' import { syncNodeProps } from './fields' +import { indexCloneSubtree, remapRepopulatedChildSources, snapshotChildSources } from './sources' function expandSeedsToParents(graph: SceneGraph, seeds: Set): Set { const expanded = new Set(seeds) @@ -134,12 +135,15 @@ export function propagateOverridesTransitively( syncNodeProps(graph, source, node, protections) if (source.childIds.length !== node.childIds.length) { + const previousSources = snapshotChildSources(graph, node.id) for (const childId of Array.from(node.childIds)) graph.deleteNode(childId) if (source.childIds.length > 0) { graph.populateInstanceChildren(node.id, sourceId, 'fig-import') + indexCloneSubtree(graph, node.id, clonesOf) } + remapRepopulatedChildSources(graph, node.id, previousSources, clonesOf) } else if (source.childIds.length > 0 && node.childIds.length > 0) { - syncChildrenDeep(graph, sourceId, node.id, swappedInstances, skip, protections) + syncChildrenDeep(graph, sourceId, node.id, swappedInstances, skip, protections, clonesOf) } syncQueue.push(cloneId) } diff --git a/packages/fig/src/instance-overrides/sync/sources.ts b/packages/fig/src/instance-overrides/sync/sources.ts new file mode 100644 index 000000000..945df2393 --- /dev/null +++ b/packages/fig/src/instance-overrides/sync/sources.ts @@ -0,0 +1,132 @@ +import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph' + +interface ChildSourceSnapshot { + id: string + path: number[] + type: SceneNode['type'] +} + +const refreshedCloneSourceMaps = new WeakSet>() + +// SceneGraph.instanceIndex contains INSTANCE nodes only, while generated text, +// frame, and vector descendants also use componentId as clone provenance. +function refreshCloneSources(graph: SceneGraph, cloneSources: Map): void { + if (refreshedCloneSourceMaps.has(cloneSources)) return + const knownIds = new Map( + [...cloneSources].map(([sourceId, cloneIds]) => [sourceId, new Set(cloneIds)]) + ) + for (const node of graph.getAllNodes()) { + if (!node.componentId) continue + let known = knownIds.get(node.componentId) + if (!known) { + known = new Set() + knownIds.set(node.componentId, known) + cloneSources.set(node.componentId, []) + } + if (known.has(node.id)) continue + known.add(node.id) + cloneSources.get(node.componentId)?.push(node.id) + } + refreshedCloneSourceMaps.add(cloneSources) +} + +export function indexCloneNodes( + graph: SceneGraph, + nodeIds: Iterable, + cloneSources: Map +): void { + const knownIds = new Map>() + for (const nodeId of nodeIds) { + const node = graph.getNode(nodeId) + if (!node?.componentId) continue + let known = knownIds.get(node.componentId) + if (!known) { + known = new Set(cloneSources.get(node.componentId)) + knownIds.set(node.componentId, known) + } + if (known.has(node.id)) continue + known.add(node.id) + const clones = cloneSources.get(node.componentId) + if (clones) clones.push(node.id) + else cloneSources.set(node.componentId, [node.id]) + } +} + +export function indexCloneSubtree( + graph: SceneGraph, + rootId: string, + cloneSources: Map +): void { + const nodeIds: string[] = [] + const queue = [rootId] + let index = 0 + while (index < queue.length) { + const node = graph.getNode(queue[index]) + index++ + if (!node) continue + nodeIds.push(node.id) + queue.push(...node.childIds) + } + indexCloneNodes(graph, nodeIds, cloneSources) +} + +/** Capture clone-source identities by path before a populated branch is replaced. */ +export function snapshotChildSources(graph: SceneGraph, parentId: string): ChildSourceSnapshot[] { + const result: ChildSourceSnapshot[] = [] + const parent = graph.getNode(parentId) + if (!parent) return result + + const visit = (nodeId: string, path: number[]) => { + const node = graph.getNode(nodeId) + if (!node) return + result.push({ id: node.id, path, type: node.type }) + node.childIds.forEach((childId, index) => visit(childId, [...path, index])) + } + parent.childIds.forEach((childId, index) => visit(childId, [index])) + return result +} + +function resolveChildPath(graph: SceneGraph, parentId: string, path: number[]): SceneNode | null { + let node = graph.getNode(parentId) + if (!node) return null + for (const index of path) { + const childId = node.childIds[index] + if (!childId) return null + node = graph.getNode(childId) + if (!node) return null + } + return node +} + +/** + * Redirect descendants that cloned the removed branch to its structural + * replacements. Without this, deep instances keep componentId references to + * deleted nodes and miss later text, visibility, and geometry synchronization. + */ +export function remapRepopulatedChildSources( + graph: SceneGraph, + parentId: string, + previousSources: ChildSourceSnapshot[], + cloneSources?: Map +): void { + if (cloneSources) refreshCloneSources(graph, cloneSources) + for (const previous of previousSources) { + const replacement = resolveChildPath(graph, parentId, previous.path) + if (!replacement || replacement.type !== previous.type) continue + const cloneIds = new Set([ + ...(cloneSources?.get(previous.id) ?? []), + ...(graph.instanceIndex.get(previous.id) ?? []) + ]) + for (const cloneId of cloneIds) { + const clone = graph.getNode(cloneId) + if (clone?.componentId !== previous.id) continue + graph.updateNode(cloneId, { componentId: replacement.id }) + if (cloneSources) { + const replacements = cloneSources.get(replacement.id) + if (replacements) { + if (!replacements.includes(cloneId)) replacements.push(cloneId) + } else cloneSources.set(replacement.id, [cloneId]) + } + } + } +} diff --git a/tests/engine/io/fig/import/component-props.test.ts b/tests/engine/io/fig/import/component-props.test.ts index 288f5762e..9c42486d8 100644 --- a/tests/engine/io/fig/import/component-props.test.ts +++ b/tests/engine/io/fig/import/component-props.test.ts @@ -105,6 +105,73 @@ describe('Figma component property import', () => { expect(unpopulatedInstance?.childIds).toEqual([]) }) + test('keeps same-name siblings without property references unchanged', () => { + const visibilityPropGuid = { sessionID: 3, localID: 3 } + const referencedIconGuid = { sessionID: 1, localID: 4 } + const plainIconGuid = { sessionID: 1, localID: 5 } + const nodeChanges: NodeChange[] = [ + { guid: documentGuid, phase: 'CREATED', type: 'DOCUMENT', name: 'Document' }, + { + guid: pageGuid, + phase: 'CREATED', + parentIndex: { guid: documentGuid, position: '!' }, + type: 'CANVAS', + name: 'Page' + }, + { + guid: componentGuid, + phase: 'CREATED', + parentIndex: { guid: pageGuid, position: '!' }, + type: 'SYMBOL', + name: 'Control', + componentPropDefs: [ + { + id: visibilityPropGuid, + name: 'Show leading icon', + type: 'BOOL', + initialValue: { boolValue: true } + } + ] + }, + { + guid: referencedIconGuid, + phase: 'CREATED', + parentIndex: { guid: componentGuid, position: '!' }, + type: 'FRAME', + name: 'Icon', + visible: true, + componentPropRefs: [{ defID: visibilityPropGuid, componentPropNodeField: 'VISIBLE' }] + }, + { + guid: plainIconGuid, + phase: 'CREATED', + parentIndex: { guid: componentGuid, position: '"' }, + type: 'FRAME', + name: 'Icon', + visible: true + }, + { + guid: instanceGuid, + phase: 'CREATED', + parentIndex: { guid: pageGuid, position: '"' }, + type: 'INSTANCE', + name: 'Control instance', + symbolData: { symbolID: componentGuid }, + componentPropAssignments: [{ defID: visibilityPropGuid, value: { boolValue: false } }] + } + ] + + const graph = importNodeChanges(nodeChanges, [], undefined, { populate: 'all' }) + const instance = Array.from(graph.getAllNodes()).find( + (node) => node.name === 'Control instance' + ) + expect(instance).toBeDefined() + expect(graph.getChildren(instance?.id ?? '').map((child) => child.visible)).toEqual([ + false, + true + ]) + }) + test('propagates nested instance swaps through clone chains', () => { const nodeChanges: NodeChange[] = [ { guid: documentGuid, phase: 'CREATED', type: 'DOCUMENT', name: 'Document' }, diff --git a/tests/engine/io/fig/import/edge-cases.test.ts b/tests/engine/io/fig/import/edge-cases.test.ts index 1d6242799..7e6e8d82f 100644 --- a/tests/engine/io/fig/import/edge-cases.test.ts +++ b/tests/engine/io/fig/import/edge-cases.test.ts @@ -394,6 +394,75 @@ describe('edge cases', () => { ) }) + test('component swaps use the component set name for variants', () => { + const graph = importNodeChanges([ + { + guid: { sessionID: 0, localID: 0 }, + type: 'DOCUMENT', + name: 'Document', + phase: 'CREATED' + } as NodeChange, + { + guid: { sessionID: 0, localID: 1 }, + parentIndex: { guid: { sessionID: 0, localID: 0 }, position: '!' }, + type: 'CANVAS', + name: 'Page', + phase: 'CREATED' + } as NodeChange, + { + guid: { sessionID: 1, localID: 1 }, + overrideKey: { sessionID: 90, localID: 1 }, + parentIndex: { guid: { sessionID: 0, localID: 1 }, position: '!' }, + type: 'SYMBOL', + name: 'Search', + phase: 'CREATED' + } as NodeChange, + { + guid: { sessionID: 1, localID: 2 }, + parentIndex: { guid: { sessionID: 0, localID: 1 }, position: '"' }, + type: 'FRAME', + name: 'Avatar', + phase: 'CREATED', + componentPropDefs: [ + { + id: { sessionID: 2, localID: 1 }, + name: 'Type', + type: 'VARIANT', + initialValue: { textValue: 'Picture' } + } + ] + } as NodeChange, + { + guid: { sessionID: 1, localID: 3 }, + parentIndex: { guid: { sessionID: 1, localID: 2 }, position: '!' }, + type: 'SYMBOL', + name: 'Type=Picture', + phase: 'CREATED' + } as NodeChange, + { + guid: { sessionID: 1, localID: 4 }, + parentIndex: { guid: { sessionID: 0, localID: 1 }, position: '#' }, + type: 'INSTANCE', + name: 'Search', + phase: 'CREATED', + symbolData: { + symbolID: { sessionID: 1, localID: 1 }, + symbolOverrides: [ + { + guidPath: { guids: [{ sessionID: 90, localID: 1 }] }, + overriddenSymbolID: { sessionID: 1, localID: 3 } + } + ] + } + } as NodeChange + ]) + + const instance = graph + .getChildren(graph.getPages()[0].id) + .find((node) => node.type === 'INSTANCE') + expect(instance?.name).toBe('Avatar') + }) + test('DSD propagates through intermediate clones that are also DSD-targeted', async () => { const graph = await parseFixture('gold-preview.fig') diff --git a/tests/engine/kiwi/instance-overrides/sync-clones.test.ts b/tests/engine/kiwi/instance-overrides/sync-clones.test.ts index f2dbfe677..21d09c711 100644 --- a/tests/engine/kiwi/instance-overrides/sync-clones.test.ts +++ b/tests/engine/kiwi/instance-overrides/sync-clones.test.ts @@ -23,11 +23,19 @@ describe('instance override clone sync', () => { componentId: targetComponent.id }) graph.populateInstanceChildren(targetChild.id, targetComponent.id) + const previousTargetLabel = graph.getChildren(targetChild.id)[0] + const downstreamLabel = graph.createNode('TEXT', page.id, { + name: 'downstream label', + text: 'Target', + componentId: previousTargetLabel.id + }) syncChildrenDeep(graph, sourceParent.id, targetParent.id, new Set()) const syncedChild = graph.getNode(targetChild.id) + const syncedLabel = graph.getChildren(targetChild.id)[0] expect(syncedChild?.componentId).toBe(sourceComponent.id) - expect(graph.getChildren(targetChild.id).map((child) => child.text)).toEqual(['Source']) + expect(syncedLabel.text).toBe('Source') + expect(graph.getNode(downstreamLabel.id)?.componentId).toBe(syncedLabel.id) }) })