582 lines
20 KiB
TypeScript
582 lines
20 KiB
TypeScript
import { describe, expect, test } from 'bun:test'
|
|
|
|
import { linkImportedInstanceChildren } from '@open-pencil/fig/node-change'
|
|
import { SceneGraph, recordInstanceOverride } from '@open-pencil/scene-graph'
|
|
|
|
describe('instance synchronization child deduplication', () => {
|
|
test('syncInstances on instance with imported children without componentId does not duplicate children', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
// Component with 1 FRAME child
|
|
const component = graph.createNode('COMPONENT', page.id, { name: 'Card' })
|
|
const compChild = graph.createNode('FRAME', component.id, {
|
|
name: 'Header',
|
|
width: 200,
|
|
height: 50
|
|
})
|
|
|
|
// Simulate an instance imported from .fig:
|
|
// The instance exists, its child exists with name 'Header', type 'FRAME',
|
|
// but its componentId is null/empty because it was imported from Figma.
|
|
const instance = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Card Instance',
|
|
componentId: component.id
|
|
})
|
|
const instChild = graph.createNode('FRAME', instance.id, {
|
|
name: 'Header',
|
|
width: 200,
|
|
height: 50,
|
|
componentId: null
|
|
})
|
|
|
|
expect(instance.childIds.length).toBe(1)
|
|
expect(instance.childIds[0]).toBe(instChild.id)
|
|
|
|
// Mutate the component child and synchronize
|
|
graph.updateNode(compChild.id, { height: 60 })
|
|
graph.syncInstances(component.id)
|
|
|
|
// In unpatched code: instance.childIds.length becomes 2 because Header was re-cloned!
|
|
expect(instance.childIds.length).toBe(1)
|
|
expect(instance.childIds[0]).toBe(instChild.id)
|
|
expect(instChild.height).toBe(60)
|
|
})
|
|
|
|
test('syncInstances handles nested imported subtrees without duplicating nested children', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
// Component hierarchy: Sidebar -> StatusFooter -> StatusRow
|
|
const sidebar = graph.createNode('COMPONENT', page.id, {
|
|
name: 'Sidebar',
|
|
width: 240,
|
|
height: 800
|
|
})
|
|
const statusFooter = graph.createNode('FRAME', sidebar.id, {
|
|
name: 'Status Footer',
|
|
width: 240,
|
|
height: 40
|
|
})
|
|
const statusRow = graph.createNode('FRAME', statusFooter.id, {
|
|
name: 'Status Row',
|
|
width: 200,
|
|
height: 24
|
|
})
|
|
|
|
// Instance hierarchy as imported from Figma (componentId is null on frames)
|
|
const sidebarInst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Sidebar Instance',
|
|
componentId: sidebar.id
|
|
})
|
|
const statusFooterInst = graph.createNode('FRAME', sidebarInst.id, {
|
|
name: 'Status Footer',
|
|
width: 240,
|
|
height: 40,
|
|
componentId: null
|
|
})
|
|
const statusRowInst = graph.createNode('FRAME', statusFooterInst.id, {
|
|
name: 'Status Row',
|
|
width: 200,
|
|
height: 24,
|
|
componentId: null
|
|
})
|
|
|
|
expect(sidebarInst.childIds.length).toBe(1)
|
|
expect(statusFooterInst.childIds.length).toBe(1)
|
|
|
|
// Mutate statusRow inside the component
|
|
graph.updateNode(statusRow.id, { height: 28, opacity: 0.8 })
|
|
graph.syncInstances(sidebar.id)
|
|
|
|
expect(sidebarInst.childIds.length).toBe(1)
|
|
expect(sidebarInst.childIds[0]).toBe(statusFooterInst.id)
|
|
expect(statusFooterInst.childIds.length).toBe(1)
|
|
expect(statusFooterInst.childIds[0]).toBe(statusRowInst.id)
|
|
expect(statusRowInst.height).toBe(28)
|
|
expect(statusRowInst.opacity).toBe(0.8)
|
|
})
|
|
|
|
test('syncInstances preserves explicit instance overrides while updating non-overridden properties', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const buttonComp = graph.createNode('COMPONENT', page.id, {
|
|
name: 'Button',
|
|
width: 100,
|
|
height: 40
|
|
})
|
|
const labelComp = graph.createNode('TEXT', buttonComp.id, {
|
|
name: 'Label',
|
|
text: 'Submit',
|
|
fontSize: 14,
|
|
width: 80,
|
|
height: 20
|
|
})
|
|
|
|
const buttonInst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Button Instance',
|
|
componentId: buttonComp.id
|
|
})
|
|
const labelInst = graph.createNode('TEXT', buttonInst.id, {
|
|
name: 'Label',
|
|
text: 'Custom Text', // override
|
|
fontSize: 14,
|
|
width: 80,
|
|
height: 20,
|
|
componentId: null
|
|
})
|
|
|
|
// Record override on text
|
|
recordInstanceOverride(graph, labelInst.id, ['text'])
|
|
|
|
// Mutate fontSize and text on component
|
|
graph.updateNode(labelComp.id, { fontSize: 16, text: 'Click Here' })
|
|
graph.syncInstances(buttonComp.id)
|
|
|
|
expect(buttonInst.childIds.length).toBe(1)
|
|
expect(labelInst.text).toBe('Custom Text') // preserved override
|
|
expect(labelInst.fontSize).toBe(16) // synced from component
|
|
})
|
|
|
|
test('syncInstances adds newly added component children while keeping existing ones', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const comp = graph.createNode('COMPONENT', page.id, { name: 'Menu' })
|
|
const _item1 = graph.createNode('FRAME', comp.id, { name: 'Item 1' })
|
|
|
|
const inst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Menu Instance',
|
|
componentId: comp.id
|
|
})
|
|
const instItem1 = graph.createNode('FRAME', inst.id, {
|
|
name: 'Item 1',
|
|
componentId: null
|
|
})
|
|
|
|
// Now add a second child to the component
|
|
const _item2 = graph.createNode('FRAME', comp.id, { name: 'Item 2', height: 32 })
|
|
graph.syncInstances(comp.id)
|
|
|
|
expect(inst.childIds.length).toBe(2)
|
|
expect(inst.childIds[0]).toBe(instItem1.id) // original child preserved
|
|
const instItem2 = graph.getNode(inst.childIds[1])
|
|
expect(instItem2?.name).toBe('Item 2')
|
|
expect(instItem2?.height).toBe(32)
|
|
})
|
|
})
|
|
|
|
describe('instance synchronization regressions from review triage', () => {
|
|
// V1: matchFallbackChildren backward iteration inverts same-name/type siblings.
|
|
test('duplicate same-name/type siblings preserve FIFO mapping during fallback sync', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const comp = graph.createNode('COMPONENT', page.id, { name: 'List', width: 200, height: 60 })
|
|
graph.createNode('FRAME', comp.id, { name: 'Row', width: 100, height: 30 })
|
|
graph.createNode('FRAME', comp.id, { name: 'Row', width: 200, height: 30 })
|
|
|
|
const inst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'List Inst',
|
|
componentId: comp.id,
|
|
width: 200,
|
|
height: 60
|
|
})
|
|
const iA = graph.createNode('FRAME', inst.id, {
|
|
name: 'Row',
|
|
width: 50,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
const iB = graph.createNode('FRAME', inst.id, {
|
|
name: 'Row',
|
|
width: 60,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
|
|
graph.syncInstances(comp.id)
|
|
|
|
expect(iA.width).toBe(100)
|
|
expect(iB.width).toBe(200)
|
|
expect(inst.childIds[0]).toBe(iA.id)
|
|
expect(inst.childIds[1]).toBe(iB.id)
|
|
})
|
|
|
|
test('duplicate same-name siblings preserve overrides on the correct child', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const comp = graph.createNode('COMPONENT', page.id, { name: 'Rows', width: 200, height: 60 })
|
|
graph.createNode('TEXT', comp.id, {
|
|
name: 'Label',
|
|
text: 'A',
|
|
fontSize: 14,
|
|
width: 80,
|
|
height: 20
|
|
})
|
|
graph.createNode('TEXT', comp.id, {
|
|
name: 'Label',
|
|
text: 'B',
|
|
fontSize: 14,
|
|
width: 80,
|
|
height: 20
|
|
})
|
|
|
|
const inst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Rows Inst',
|
|
componentId: comp.id,
|
|
width: 200,
|
|
height: 60
|
|
})
|
|
const labelA = graph.createNode('TEXT', inst.id, {
|
|
name: 'Label',
|
|
text: 'override-A',
|
|
fontSize: 14,
|
|
width: 80,
|
|
height: 20,
|
|
componentId: null
|
|
})
|
|
const labelB = graph.createNode('TEXT', inst.id, {
|
|
name: 'Label',
|
|
text: 'override-B',
|
|
fontSize: 14,
|
|
width: 80,
|
|
height: 20,
|
|
componentId: null
|
|
})
|
|
|
|
recordInstanceOverride(graph, labelA.id, ['text'])
|
|
|
|
const compLabels = comp.childIds
|
|
.map((id) => graph.getNode(id))
|
|
.filter((n): n is NonNullable<typeof n> => n !== undefined)
|
|
graph.updateNode(compLabels[0].id, { text: 'A-changed' })
|
|
graph.updateNode(compLabels[1].id, { text: 'B-changed' })
|
|
|
|
graph.syncInstances(comp.id)
|
|
|
|
expect(labelA.text).toBe('override-A')
|
|
expect(labelB.text).toBe('B-changed')
|
|
})
|
|
|
|
test('same-name extra beside a renamed imported child remains untouched', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
const component = graph.createNode('COMPONENT', page.id, { name: 'List' })
|
|
const row = graph.createNode('FRAME', component.id, { name: 'Row', width: 100 })
|
|
const instance = graph.createNode('INSTANCE', page.id, {
|
|
name: 'List',
|
|
componentId: component.id
|
|
})
|
|
const extra = graph.createNode('FRAME', instance.id, { name: 'Row', width: 20 })
|
|
const renamed = graph.createNode('FRAME', instance.id, { name: 'Row v2', width: 100 })
|
|
|
|
graph.syncInstances(component.id)
|
|
|
|
expect(extra.width).toBe(20)
|
|
expect(extra.componentId).toBeNull()
|
|
expect(renamed.name).toBe('Row v2')
|
|
expect(renamed.componentId).toBeNull()
|
|
const mapped = graph.getChildren(instance.id).filter((child) => child.componentId === row.id)
|
|
expect(mapped).toHaveLength(1)
|
|
expect(mapped[0]?.id).not.toBe(extra.id)
|
|
})
|
|
|
|
// V2+V3: extra unmatched instance child is not co-opted by positional or type-only fallback.
|
|
test('extra unmatched instance child is not co-opted or renamed by fallback matching', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const comp = graph.createNode('COMPONENT', page.id, { name: 'Menu', width: 200, height: 60 })
|
|
graph.createNode('FRAME', comp.id, { name: 'Item 1', width: 100, height: 30 })
|
|
|
|
const inst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Menu Inst',
|
|
componentId: comp.id,
|
|
width: 200,
|
|
height: 60
|
|
})
|
|
const instItem1 = graph.createNode('FRAME', inst.id, {
|
|
name: 'Item 1',
|
|
width: 100,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
const extra = graph.createNode('FRAME', inst.id, {
|
|
name: 'Badge',
|
|
width: 20,
|
|
height: 20,
|
|
componentId: null
|
|
})
|
|
|
|
graph.createNode('FRAME', comp.id, { name: 'Item 2', width: 100, height: 32 })
|
|
|
|
graph.syncInstances(comp.id)
|
|
|
|
expect(extra.name).toBe('Badge')
|
|
expect(extra.width).toBe(20)
|
|
expect(extra.height).toBe(20)
|
|
expect(inst.childIds.length).toBe(3)
|
|
expect(inst.childIds[0]).toBe(instItem1.id)
|
|
expect(inst.childIds).toContain(extra.id)
|
|
})
|
|
|
|
// Sort tie-break contract: mapped children sort in component order, unmapped extras
|
|
// sort to the END (not the front). Locks the deliberate behavior change where the
|
|
// comparator sends unmapped children (no componentId resolving to a comp child) to
|
|
// the end instead of the front.
|
|
test('unmapped extra children sort to the end after sync, mapped children keep component order', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const comp = graph.createNode('COMPONENT', page.id, { name: 'Menu', width: 200, height: 60 })
|
|
graph.createNode('FRAME', comp.id, { name: 'Item 1', width: 100, height: 30 })
|
|
const compItem2 = graph.createNode('FRAME', comp.id, { name: 'Item 2', width: 100, height: 32 })
|
|
|
|
const inst = graph.createNode('INSTANCE', page.id, {
|
|
name: 'Menu Inst',
|
|
componentId: comp.id,
|
|
width: 200,
|
|
height: 60
|
|
})
|
|
const instItem1 = graph.createNode('FRAME', inst.id, {
|
|
name: 'Item 1',
|
|
width: 100,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
const extra = graph.createNode('FRAME', inst.id, {
|
|
name: 'Badge',
|
|
width: 20,
|
|
height: 20,
|
|
componentId: null
|
|
})
|
|
|
|
graph.syncInstances(comp.id)
|
|
|
|
expect(inst.childIds.length).toBe(3)
|
|
// Mapped children in component child order first...
|
|
expect(inst.childIds[0]).toBe(instItem1.id)
|
|
const instItem2 = graph.getNode(inst.childIds[1])
|
|
expect(instItem2?.name).toBe('Item 2')
|
|
expect(instItem2?.componentId).toBe(compItem2.id)
|
|
// ...unmapped extra LAST — not yanked to the front on every sync.
|
|
expect(inst.childIds[2]).toBe(extra.id)
|
|
})
|
|
|
|
// V5: self-referential component does not cause unbounded recursion / OOM.
|
|
test('self-referential component does not hang or OOM on syncInstances', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const comp = graph.createNode('COMPONENT', page.id, { name: 'Self', width: 100, height: 50 })
|
|
graph.createNode('INSTANCE', comp.id, {
|
|
name: 'Self Inst',
|
|
componentId: comp.id,
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
|
|
graph.syncInstances(comp.id)
|
|
|
|
expect(graph.countDescendants(comp.id)).toBeLessThan(50)
|
|
})
|
|
|
|
test('syncInstances on component with instance whose componentId is an ancestor is bounded', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const outer = graph.createNode('COMPONENT', page.id, { name: 'Outer', width: 200, height: 100 })
|
|
const inner = graph.createNode('COMPONENT', outer.id, { name: 'Inner', width: 100, height: 50 })
|
|
graph.createNode('INSTANCE', inner.id, {
|
|
name: 'Outer Inst',
|
|
componentId: outer.id,
|
|
width: 200,
|
|
height: 100
|
|
})
|
|
|
|
graph.syncInstances(outer.id)
|
|
graph.syncInstances(inner.id)
|
|
|
|
expect(graph.countDescendants(outer.id)).toBeLessThan(100)
|
|
})
|
|
|
|
// V7: graph-scoped syncingComponents. GENUINE reentrancy test — unlike the old
|
|
// two-sequential-calls version (which passed even with NO guard), this fires the
|
|
// reentrancy guard from inside a sync via the node:created event (emitted when
|
|
// syncChildren Pass 3 clones a missing component child), and proves a second graph
|
|
// with the same componentId can sync while the first is mid-sync — the exact
|
|
// property a module-global Set breaks.
|
|
test('reentrant syncInstances is a no-op, and a second graph with the same component ID syncs mid-sync', () => {
|
|
const graph1 = new SceneGraph()
|
|
const page1 = graph1.getPages()[0]
|
|
if (!page1) throw new Error('page1 missing')
|
|
graph1.createNodeWithId('comp-x', 'COMPONENT', page1.id, { name: 'C', width: 100, height: 50 })
|
|
// Component child so Pass 3 clones it into inst-x — the clone fires node:created
|
|
// mid-sync, which is where the reentrancy is exercised from.
|
|
graph1.createNodeWithId('comp-row', 'FRAME', 'comp-x', { name: 'Row', width: 100, height: 10 })
|
|
graph1.createNodeWithId('inst-x', 'INSTANCE', page1.id, {
|
|
name: 'I',
|
|
componentId: 'comp-x',
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
|
|
const graph2 = new SceneGraph()
|
|
const page2 = graph2.getPages()[0]
|
|
if (!page2) throw new Error('page2 missing')
|
|
graph2.createNodeWithId('comp-x', 'COMPONENT', page2.id, { name: 'C', width: 80, height: 40 })
|
|
graph2.createNodeWithId('inst-x', 'INSTANCE', page2.id, {
|
|
name: 'I',
|
|
componentId: 'comp-x',
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
|
|
let firedDuringSync = 0
|
|
let graph2SyncedDuringSync = false
|
|
|
|
// Arm AFTER both graphs exist, so the handler only fires for nodes created
|
|
// during graph1's sync (the Pass 3 clone).
|
|
const unbind = graph1.onNodeEvents({
|
|
created: () => {
|
|
// Re-entrant sync of the SAME graph+component must be safe: the guard makes
|
|
// it a no-op, and even a hypothetical non-guarded re-entry must stay
|
|
// idempotent (the clone already carries componentId, so Pass 1 matches it
|
|
// and nothing is re-cloned). Call it to prove neither path corrupts state.
|
|
graph1.syncInstances('comp-x')
|
|
|
|
// Cross-graph sync with the SAME componentId must proceed (WeakMap scoping).
|
|
graph2.syncInstances('comp-x')
|
|
firedDuringSync++
|
|
if (graph2.getNode('inst-x')?.width === 80) graph2SyncedDuringSync = true
|
|
}
|
|
})
|
|
|
|
try {
|
|
graph1.syncInstances('comp-x')
|
|
} finally {
|
|
unbind()
|
|
}
|
|
|
|
// The handler fired at least once during the sync (reentrancy actually exercised —
|
|
// this is what makes the test non-vacuous).
|
|
expect(firedDuringSync).toBeGreaterThan(0)
|
|
// The re-entrant graph1 sync left exactly one clone of comp-row (no corruption).
|
|
expect(graph1.getNode('inst-x')?.childIds.length).toBe(1)
|
|
// graph2's instance synced while graph1's sync was in progress — a module-global
|
|
// Set would have blocked this ('comp-x' already marked in-flight). This is the
|
|
// discriminating assertion: the old sequential-calls test passed even with a
|
|
// module-global Set; this one does not.
|
|
expect(graph2SyncedDuringSync).toBe(true)
|
|
expect(graph2.getNode('inst-x')?.width).toBe(80)
|
|
// The outer sync still completed correctly: inst1 synced its component's width.
|
|
expect(graph1.getNode('inst-x')?.width).toBe(100)
|
|
})
|
|
|
|
// V4: import-linkage does not stamp sub-instance descendants with proxy componentIds.
|
|
test('linkImportedInstanceChildren does not recurse across INSTANCE boundaries', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const compB = graph.createNode('COMPONENT', page.id, { name: 'B', width: 100, height: 50 })
|
|
const compBChild = graph.createNode('FRAME', compB.id, { name: 'G', width: 80, height: 30 })
|
|
|
|
const compA = graph.createNode('COMPONENT', page.id, { name: 'A', width: 200, height: 100 })
|
|
const compA_F = graph.createNode('FRAME', compA.id, { name: 'F', width: 200, height: 100 })
|
|
const compA_N = graph.createNode('INSTANCE', compA_F.id, {
|
|
name: 'N',
|
|
componentId: compB.id,
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
const compA_N_Child = graph.createNode('FRAME', compA_N.id, {
|
|
name: 'G',
|
|
width: 80,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
|
|
const instA = graph.createNode('INSTANCE', page.id, {
|
|
name: 'A Inst',
|
|
componentId: compA.id,
|
|
width: 200,
|
|
height: 100
|
|
})
|
|
const instA_F = graph.createNode('FRAME', instA.id, {
|
|
name: 'F',
|
|
width: 200,
|
|
height: 100,
|
|
componentId: null
|
|
})
|
|
const instA_N = graph.createNode('INSTANCE', instA_F.id, {
|
|
name: 'N',
|
|
componentId: compB.id,
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
const instA_N_Child = graph.createNode('FRAME', instA_N.id, {
|
|
name: 'G',
|
|
width: 80,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
|
|
linkImportedInstanceChildren(graph)
|
|
|
|
expect(instA_N_Child.componentId).toBe(compBChild.id)
|
|
expect(instA_N_Child.componentId).not.toBe(compA_N_Child.id)
|
|
})
|
|
|
|
test('syncInstances on sub-instance main component works after linkImportedInstanceChildren', () => {
|
|
const graph = new SceneGraph()
|
|
const page = graph.addPage('Page')
|
|
|
|
const compB = graph.createNode('COMPONENT', page.id, { name: 'B', width: 100, height: 50 })
|
|
const compBChild = graph.createNode('FRAME', compB.id, { name: 'G', width: 80, height: 30 })
|
|
|
|
const compA = graph.createNode('COMPONENT', page.id, { name: 'A', width: 200, height: 100 })
|
|
const compA_F = graph.createNode('FRAME', compA.id, { name: 'F', width: 200, height: 100 })
|
|
graph.createNode('INSTANCE', compA_F.id, {
|
|
name: 'N',
|
|
componentId: compB.id,
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
|
|
const instA = graph.createNode('INSTANCE', page.id, {
|
|
name: 'A Inst',
|
|
componentId: compA.id,
|
|
width: 200,
|
|
height: 100
|
|
})
|
|
const instA_F = graph.createNode('FRAME', instA.id, {
|
|
name: 'F',
|
|
width: 200,
|
|
height: 100,
|
|
componentId: null
|
|
})
|
|
const instA_N = graph.createNode('INSTANCE', instA_F.id, {
|
|
name: 'N',
|
|
componentId: compB.id,
|
|
width: 100,
|
|
height: 50
|
|
})
|
|
const instA_N_Child = graph.createNode('FRAME', instA_N.id, {
|
|
name: 'G',
|
|
width: 80,
|
|
height: 30,
|
|
componentId: null
|
|
})
|
|
|
|
linkImportedInstanceChildren(graph)
|
|
|
|
graph.updateNode(compBChild.id, { height: 40 })
|
|
graph.syncInstances(compB.id)
|
|
|
|
expect(instA_N_Child.height).toBe(40)
|
|
})
|
|
})
|