diff --git a/packages/core/src/kiwi/instance-overrides.ts b/packages/core/src/kiwi/instance-overrides.ts index 72d64bd9e..0533de4cc 100644 --- a/packages/core/src/kiwi/instance-overrides.ts +++ b/packages/core/src/kiwi/instance-overrides.ts @@ -267,9 +267,17 @@ export function populateAndApplyOverrides( const node = graph.getNode(nodeId) if (node?.type !== 'INSTANCE') return + // Only rename when the current name matches the root component name + // (i.e. it wasn't manually overridden by the user). + const rootCompId = node.componentId ? getComponentRoot(node.componentId) : undefined + const rootComp = rootCompId ? graph.getNode(rootCompId) : undefined for (const childId of Array.from(node.childIds)) graph.deleteNode(childId) - graph.updateNode(nodeId, { componentId: compId }) const comp = graph.getNode(compId) + const updates: Partial = { componentId: compId } + if (comp?.name && rootComp?.name && node.name === rootComp.name) { + updates.name = comp.name + } + graph.updateNode(nodeId, updates) if (comp && comp.childIds.length > 0) { graph.populateInstanceChildren(nodeId, compId) } @@ -532,6 +540,11 @@ export function populateAndApplyOverrides( propagateDsdChanges(dsdModified, dsdSizeSet) } + // Tracks INSTANCE nodes whose componentId was changed by a swap override. + // Populated in applySymbolOverrides and recloneChildren, read in syncChildrenDeep + // to propagate swaps transitively through clone chains. + const swappedInstances = new Set() + function applySymbolOverrides(): Set { const overriddenNodes = new Set() componentIdRoot.clear() @@ -556,7 +569,10 @@ export function populateAndApplyOverrides( if (ov.overriddenSymbolID) { const swapGuid = guidToString(ov.overriddenSymbolID) const newCompId = guidToNodeId.get(swapGuid) - if (newCompId) repopulateInstance(targetId, newCompId) + if (newCompId) { + repopulateInstance(targetId, newCompId) + swappedInstances.add(targetId) + } } const { guidPath: _, overriddenSymbolID: _s, componentPropAssignments: _c, ...fields } = ov @@ -586,6 +602,19 @@ export function populateAndApplyOverrides( if (Object.keys(updates).length > 0) graph.updateNode(target.id, updates) } + function recloneChildren(srcChildId: string, tgtNode: SceneNode) { + const srcChild = graph.getNode(srcChildId) + if (!srcChild) return + + for (const childId of [...tgtNode.childIds]) graph.deleteNode(childId) + graph.updateNode(tgtNode.id, { name: srcChild.name, componentId: srcChild.componentId }) + syncNodeProps(srcChild, tgtNode) + if (srcChild.childIds.length > 0) { + graph.populateInstanceChildren(tgtNode.id, srcChildId) + } + swappedInstances.add(tgtNode.id) + } + function syncChildrenDeep(sourceId: string, targetId: string, skip?: Set) { const src = graph.getNode(sourceId) const tgt = graph.getNode(targetId) @@ -596,6 +625,12 @@ export function populateAndApplyOverrides( const srcNode = graph.getNode(src.childIds[i]) const tgtNode = graph.getNode(tgt.childIds[i]) if (!srcNode || !tgtNode || srcNode.type !== tgtNode.type) continue + + if (srcNode.type === 'INSTANCE' && swappedInstances.has(src.childIds[i]) && srcNode.componentId !== tgtNode.componentId) { + recloneChildren(src.childIds[i], tgtNode) + continue + } + syncNodeProps(srcNode, tgtNode) syncChildrenDeep(src.childIds[i], tgt.childIds[i], skip) } @@ -662,6 +697,8 @@ export function populateAndApplyOverrides( function propagateOverridesTransitively(seeds: Set) { if (seeds.size === 0) return + // Stale after applySymbolOverrides changed componentIds via repopulateInstance + componentIdRoot.clear() const clonesOf = buildClonesMap() const expandedSeeds = expandSeedsToParents(seeds) const needsSync = buildNeedsSyncSet(expandedSeeds, clonesOf) diff --git a/tests/engine/instance-overrides.test.ts b/tests/engine/instance-overrides.test.ts new file mode 100644 index 000000000..a2853dec4 --- /dev/null +++ b/tests/engine/instance-overrides.test.ts @@ -0,0 +1,241 @@ +import { describe, test, expect } from 'bun:test' + +import { importNodeChanges, type NodeChange } from '@open-pencil/core' + +const ID = { m00: 1, m01: 0, m02: 0, m10: 0, m11: 1, m12: 0 } +const SIZE = { x: 100, y: 100 } + +function nc( + sessionID: number, + localID: number, + type: string, + parentSessionID: number, + parentLocalID: number, + name: string, + extra: Record = {} +): NodeChange { + return { + guid: { sessionID, localID }, + parentIndex: { + guid: { sessionID: parentSessionID, localID: parentLocalID }, + position: String.fromCharCode(33 + localID), + }, + type, + name, + visible: true, + opacity: 1, + phase: 'CREATED', + size: SIZE, + transform: ID, + ...extra, + } as NodeChange +} + +function guid(s: number, l: number) { + return { sessionID: s, localID: l } +} + +/** + * Build a minimal fig structure for testing instance swap overrides: + * + * Document (0:0) + * ├── InternalPage (0:1) [internalOnly] + * │ ├── IconA COMPONENT (0:10) — children: VectorA (0:11) + * │ ├── IconB COMPONENT (0:20) — children: VectorB (0:21) + * │ └── Button COMPONENT (0:30) — children: Icon INSTANCE→IconA (0:31) + * └── Page1 (0:2) + * └── ButtonInstance INSTANCE→Button (0:40) + * symbolOverride: swap Icon to IconB + */ +function swapOverrideFixture(opts?: { customName?: string }): NodeChange[] { + return [ + nc(0, 0, 'DOCUMENT', 0, 0, 'Document'), + nc(0, 1, 'CANVAS', 0, 0, 'InternalPage', { internalOnly: true }), + nc(0, 2, 'CANVAS', 0, 0, 'Page1'), + + // IconA component with a vector child + nc(0, 10, 'COMPONENT', 0, 1, 'IconA'), + nc(0, 11, 'VECTOR', 0, 10, 'VectorA'), + + // IconB component with a vector child + nc(0, 20, 'COMPONENT', 0, 1, 'IconB'), + nc(0, 21, 'VECTOR', 0, 20, 'VectorB'), + + // Button component containing an Icon instance pointing to IconA + nc(0, 30, 'COMPONENT', 0, 1, 'Button'), + nc(0, 31, 'INSTANCE', 0, 30, opts?.customName ?? 'IconA', { + symbolData: { symbolID: guid(0, 10) }, + }), + + // Visible-page instance of Button with a swap override: Icon→IconB + nc(0, 40, 'INSTANCE', 0, 2, 'ButtonInstance', { + symbolData: { + symbolID: guid(0, 30), + symbolOverrides: [ + { + guidPath: { guids: [guid(0, 31)] }, + overriddenSymbolID: guid(0, 20), + }, + ], + }, + }), + ] +} + +describe('instance swap overrides', () => { + test('swap override renames icon and reclones children', () => { + const graph = importNodeChanges(swapOverrideFixture()) + const page = graph.getPages().find((p) => p.name === 'Page1')! + const button = graph.getChildren(page.id)[0] + expect(button.name).toBe('ButtonInstance') + + const icon = graph.getChildren(button.id)[0] + expect(icon.name).toBe('IconB') + expect(icon.type).toBe('INSTANCE') + + const iconChildren = graph.getChildren(icon.id) + expect(iconChildren.length).toBe(1) + expect(iconChildren[0].name).toBe('VectorB') + }) + + test('swap override preserves user-given name', () => { + const graph = importNodeChanges(swapOverrideFixture({ customName: 'MyCustomIcon' })) + const page = graph.getPages().find((p) => p.name === 'Page1')! + const button = graph.getChildren(page.id)[0] + const icon = graph.getChildren(button.id)[0] + + // Name was "MyCustomIcon" which doesn't match root component "IconA", + // so it should NOT be renamed to "IconB" + expect(icon.name).toBe('MyCustomIcon') + + // Children should still be swapped to IconB's children + const iconChildren = graph.getChildren(icon.id) + expect(iconChildren.length).toBe(1) + expect(iconChildren[0].name).toBe('VectorB') + }) + + test('swap propagates transitively through 2-level clone chain', () => { + // Add a second instance that clones the first button instance + const nodes = [ + ...swapOverrideFixture(), + // Wrapper COMPONENT on internal page that contains a Button instance + nc(0, 50, 'COMPONENT', 0, 1, 'Wrapper'), + nc(0, 51, 'INSTANCE', 0, 50, 'ButtonInstance', { + symbolData: { symbolID: guid(0, 30) }, + }), + // WrapperInstance on visible page with swap override on the button's icon + nc(0, 60, 'INSTANCE', 0, 2, 'WrapperInstance', { + symbolData: { + symbolID: guid(0, 50), + symbolOverrides: [ + { + guidPath: { guids: [guid(0, 51), guid(0, 31)] }, + overriddenSymbolID: guid(0, 20), + }, + ], + }, + }), + ] + + const graph = importNodeChanges(nodes) + const page = graph.getPages().find((p) => p.name === 'Page1')! + const children = graph.getChildren(page.id) + + const wrapper = children.find((c) => c.name === 'WrapperInstance')! + expect(wrapper).toBeDefined() + + // WrapperInstance > ButtonInstance > icon + const wrapperButton = graph.getChildren(wrapper.id)[0] + expect(wrapperButton.name).toBe('ButtonInstance') + + const icon = graph.getChildren(wrapperButton.id)[0] + expect(icon.name).toBe('IconB') + + const iconChildren = graph.getChildren(icon.id) + expect(iconChildren.length).toBe(1) + expect(iconChildren[0].name).toBe('VectorB') + }) + + test('swap propagates to sibling clone at same level', () => { + // Two Button instances on the visible page, both cloning the same component. + // Only one has a swap override — the other should keep the default icon. + const nodes: NodeChange[] = [ + ...swapOverrideFixture(), + // Second ButtonInstance with NO swap override + nc(0, 41, 'INSTANCE', 0, 2, 'ButtonDefault', { + symbolData: { symbolID: guid(0, 30) }, + }), + ] + + const graph = importNodeChanges(nodes) + const page = graph.getPages().find((p) => p.name === 'Page1')! + const children = graph.getChildren(page.id) + + const swapped = children.find((c) => c.name === 'ButtonInstance')! + const swappedIcon = graph.getChildren(swapped.id)[0] + expect(swappedIcon.name).toBe('IconB') + + const defaultBtn = children.find((c) => c.name === 'ButtonDefault')! + const defaultIcon = graph.getChildren(defaultBtn.id)[0] + expect(defaultIcon.name).toBe('IconA') + }) + + test('transitive propagation: clone of swapped clone gets swap', () => { + // ButtonInstance (swap Icon→IconB) on internal page, + // then ButtonClone on visible page clones ButtonInstance. + // ButtonClone's icon should also be IconB. + const nodes: NodeChange[] = [ + nc(0, 0, 'DOCUMENT', 0, 0, 'Document'), + nc(0, 1, 'CANVAS', 0, 0, 'InternalPage', { internalOnly: true }), + nc(0, 2, 'CANVAS', 0, 0, 'Page1'), + + nc(0, 10, 'COMPONENT', 0, 1, 'IconA'), + nc(0, 11, 'VECTOR', 0, 10, 'VectorA'), + nc(0, 20, 'COMPONENT', 0, 1, 'IconB'), + nc(0, 21, 'VECTOR', 0, 20, 'VectorB'), + + // Button component: children = Icon INSTANCE→IconA + nc(0, 30, 'COMPONENT', 0, 1, 'Button'), + nc(0, 31, 'INSTANCE', 0, 30, 'IconA', { + symbolData: { symbolID: guid(0, 10) }, + }), + + // ButtonSwapped: instance of Button on internal page with swap + nc(0, 40, 'INSTANCE', 0, 1, 'ButtonSwapped', { + symbolData: { + symbolID: guid(0, 30), + symbolOverrides: [ + { + guidPath: { guids: [guid(0, 31)] }, + overriddenSymbolID: guid(0, 20), + }, + ], + }, + }), + + // Container component on internal page wrapping ButtonSwapped + nc(0, 50, 'COMPONENT', 0, 1, 'Container'), + nc(0, 51, 'INSTANCE', 0, 50, 'ButtonSwapped', { + symbolData: { symbolID: guid(0, 40) }, + }), + + // Visible-page instance of Container + nc(0, 60, 'INSTANCE', 0, 2, 'ContainerInstance', { + symbolData: { symbolID: guid(0, 50) }, + }), + ] + + const graph = importNodeChanges(nodes) + const page = graph.getPages().find((p) => p.name === 'Page1')! + const container = graph.getChildren(page.id)[0] + + // ContainerInstance > ButtonSwapped > icon + const button = graph.getChildren(container.id)[0] + const icon = graph.getChildren(button.id)[0] + expect(icon.name).toBe('IconB') + + const iconChildren = graph.getChildren(icon.id) + expect(iconChildren.length).toBe(1) + expect(iconChildren[0].name).toBe('VectorB') + }) +})