fix(fig): keep the sizes Figma derived for instance layers when a page loads (#951)

* fix(fig): keep the sizes Figma derived for instance layers when a page loads

Loading a page resynced every component its new instances used, which copied component sizes over the sizes Figma derived for instance layers on earlier pages, and over those of the page's own new instances. A text field filling a 312px instance came back at its component's 210px.

The page now syncs only the instances it placed, through a new SceneGraph.syncInstance, and then puts back the sizes Figma derived for them unless that component layer's size was edited live. The gold input now matches the captured Figma Content width (374.86 instead of 633.69), and its placeholder matches the 97.51 x 39.38 Figma derived in the file.

* fix(fig): keep live edits to mapped component layers over derived sizes

restoreDerivedSizes checked only a layer's own component for live edits. A
nested instance clone syncs from the layer its owning instance maps it to
through sourceComponentId, so a live width edit to that layer was carried
over by the sync and then replaced by the size Figma derived. The restore
now reads the same source layers as reconcileLiveComponentEdits, through a
shared syncSourceLayers helper.

packages/core/tests/io/formats/fig/session/page-instance-sync.test.ts

@@ -5,12 +5,76 @@ import { initCodec } from '@open-pencil/core/kiwi'
 import { createFigDocumentSession } from '@open-pencil/fig'
 import { SceneGraph } from '@open-pencil/scene-graph'
 
+test('loading a later page keeps the derived sizes of instances earlier pages placed', async () => {
+  await initCodec()
+  const source = new SceneGraph()
+  const first = source.getPages()[0]
+  const component = source.createNode('COMPONENT', first.id, { name: 'Field', width: 200 })
+  source.createNode('RECTANGLE', component.id, { name: 'Box', width: 100, height: 20 })
+  const placed = source.createInstance(component.id, first.id)
+  if (!placed) throw new Error('Missing instance')
+  // The instance's box is wider than the component's, as a fill child of a resized instance is;
+  // export saves that as the size Figma derived for it.
+  const box = source.getChildren(placed.id)[0]
+  source.updateNode(box.id, { width: 150 })
+  source.createInstance(component.id, source.addPage('Second').id)
+  const bytes = await exportFigFile(source)
+
+  // The app's sessions read the sizes Figma derived, as this one does.
+  const session = createFigDocumentSession(bytes.buffer as ArrayBuffer, { derivedBounds: true })
+  session.loadPage(session.pages[0].id)
+  const firstPageId = session.graphPageId(session.pages[0].id)
+  const instance = session.graph
+    .getChildren(firstPageId ?? '')
+    .find((node) => node.type === 'INSTANCE')
+  if (!instance) throw new Error('Missing loaded instance')
+  const loadedBox = () => session.graph.getChildren(instance.id)[0]
+  expect(loadedBox().width).toBe(150)
+
+  session.loadPage(session.pages[1].id)
+  expect(loadedBox().width).toBe(150)
+})
+
+test('loading a later page keeps a live edit to a nested instance layer over its derived size', async () => {
+  await initCodec()
+  const source = new SceneGraph()
+  const library = source.getPages()[0]
+  const inner = source.createNode('COMPONENT', library.id, { name: 'Inner', width: 100 })
+  source.createNode('RECTANGLE', inner.id, { name: 'Box', width: 100, height: 20 })
+  const outer = source.createNode('COMPONENT', library.id, { name: 'Outer', width: 300 })
+  const layer = source.createInstance(inner.id, outer.id)
+  if (!layer) throw new Error('Missing nested instance layer')
+  const placed = source.createInstance(outer.id, source.addPage('Placed').id)
+  if (!placed) throw new Error('Missing placed instance')
+  // Figma derived a wider nested instance in the placed copy, as a fill child would be.
+  const nested = source.getChildren(placed.id)[0]
+  source.updateNode(nested.id, { width: 150 })
+  const bytes = await exportFigFile(source)
+
+  const session = createFigDocumentSession(bytes.buffer as ArrayBuffer, { derivedBounds: true })
+  session.loadPage(session.pages[0].id)
+  const libraryId = session.graphPageId(session.pages[0].id) ?? ''
+  const loadedOuter = session.graph.getChildren(libraryId).find((node) => node.name === 'Outer')
+  const loadedLayer = loadedOuter ? session.graph.getChildren(loadedOuter.id)[0] : undefined
+  if (!loadedLayer) throw new Error('Missing loaded nested instance layer')
+  // Editing the component's layer live, after the file opened.
+  session.graph.updateNode(loadedLayer.id, { width: 120 })
+  expect(session.graph.getNode(loadedLayer.id)?.source.editedFields).toContain('width')
+
+  const placedPage = session.pages.find((page) => page.name === 'Placed')
+  if (!placedPage) throw new Error('Missing placed page')
+  session.loadPage(placedPage.id)
+  const placedId = session.graphPageId(placedPage.id) ?? ''
+  const loadedPlaced = session.graph.getChildren(placedId)[0]
+  expect(session.graph.getChildren(loadedPlaced.id)[0].width).toBe(120)
+})
+
 /**
- * Syncing visits every instance of a component, so doing it per placed instance costs a
- * pass over the whole graph for each one. A page placing many instances of one component
- * must still sync it once.
+ * Syncing a whole component visits every instance of it in the graph, including those earlier
+ * pages placed, whose sizes Figma derived. A resumed page syncs only the instances it places,
+ * each once.
  */
-test('a resumed page load syncs each component once, not once per instance it places', async () => {
+test('a resumed page load syncs each instance it places once, never a whole component', async () => {
   await initCodec()
   const source = new SceneGraph()
   const library = source.getPages()[0]
@@ -22,21 +86,27 @@ test('a resumed page load syncs each component once, not once per instance it pl
 
   const session = createFigDocumentSession(bytes.buffer as ArrayBuffer)
   const synced: string[] = []
+  const wholeComponents: string[] = []
+  const syncInstance = session.graph.syncInstance.bind(session.graph)
+  session.graph.syncInstance = (instanceId: string) => {
+    synced.push(instanceId)
+    return syncInstance(instanceId)
+  }
   const syncInstances = session.graph.syncInstances.bind(session.graph)
   session.graph.syncInstances = (componentId: string) => {
-    synced.push(componentId)
+    wholeComponents.push(componentId)
     return syncInstances(componentId)
   }
   const placed = session.pages.find((candidate) => candidate.name === 'Placed')
   if (!placed) throw new Error('Missing placed page')
   session.loadPage(placed.id)
 
-  // One component placed eight times is one sync, not eight.
-  expect(synced.length).toBe(1)
   const pageId = session.graphPageId(placed.id)
   if (!pageId) throw new Error('Missing materialized page')
   const instances = session.graph.getChildren(pageId)
   expect(instances).toHaveLength(8)
+  expect(wholeComponents).toEqual([])
+  expect(synced.toSorted()).toEqual(instances.map((instance) => instance.id).toSorted())
   for (const instance of instances)
     expect(session.graph.getChildren(instance.id)[0].text).toBe('Badge')
 })

packages/fig/src/document/materialize.ts

@@ -1,7 +1,10 @@
 import type { NodeChange } from '@open-pencil/kiwi/fig/codec'
 import { SceneGraph, type SceneNode } from '@open-pencil/scene-graph'
 
-import { reconcileLiveComponentEdits } from '../instance-overrides/live-component-edits'
+import {
+  reconcileLiveComponentEdits,
+  syncSourceLayers
+} from '../instance-overrides/live-component-edits'
 import { materializeInstance } from '../instance-overrides/materialize-instance'
 import type {
   InstanceOccurrence,
@@ -169,6 +172,31 @@ function createAssemblyState(
   }
 }
 
+/**
+ * Syncing a resumed load's new instances copies their component's sizes over the sizes Figma
+ * derived for them, such as a field filling its resized instance. Puts the derived sizes back,
+ * except along an axis whose component layer was edited live, which syncing rightly carried over:
+ * the layer an owning instance maps it to, or its own component.
+ */
+function restoreDerivedSizes(
+  graph: SceneGraph,
+  derivedSizes: ReadonlyMap<string, { width: number; height: number }>
+): void {
+  graph.preserveSourceMetadataDuring(() => {
+    for (const [id, size] of derivedSizes) {
+      const node = graph.getNode(id)
+      if (!node) continue
+      const edited = new Set(
+        syncSourceLayers(graph, node).flatMap((layer) => layer.source.editedFields)
+      )
+      const updates: Partial<SceneNode> = {}
+      if (node.width !== size.width && !edited.has('width')) updates.width = size.width
+      if (node.height !== size.height && !edited.has('height')) updates.height = size.height
+      if (Object.keys(updates).length > 0) graph.updateNode(id, updates)
+    }
+  })
+}
+
 function materializeReader(
   reader: ReturnType<typeof createDocumentReader>,
   blobs: Uint8Array[],
@@ -186,9 +214,14 @@ function materializeReader(
   const { graph, sources, components, savedSizeNodes, componentIds } = state
   const existingNodeIds = new Set(graph.nodes.keys())
   const layoutScales = new Map<string, number>()
+  /** The sizes Figma derived for this pass's instance layers, as materialized. */
+  const derivedSizes = new Map<string, { width: number; height: number }>()
   const rememberDerivedSizes = (nodes: ReadonlyMap<InstanceOccurrence, SceneNode>): void => {
     for (const [occurrence, node] of nodes) {
-      if (occurrence.derivedSize) savedSizeNodes.add(node.id)
+      if (occurrence.derivedSize) {
+        savedSizeNodes.add(node.id)
+        derivedSizes.set(node.id, { width: node.width, height: node.height })
+      }
       if (occurrence.layoutScale !== undefined) layoutScales.set(node.id, occurrence.layoutScale)
     }
   }
@@ -232,9 +265,9 @@ function materializeReader(
     sources.set(item.sourceId, materialized.root.id)
   }
   /**
-   * Components whose instances a resumed load must re-sync. Syncing visits every instance
-   * of a component, so a page placing many instances of one component collects it once
-   * and syncs after the page is built rather than per instance.
+   * Instances a resumed load must sync with their components, which may have changed live since
+   * the file loaded. Only this page's new instances: syncing a whole component would copy its
+   * sizes over the sizes Figma derived for instances earlier pages already placed.
    */
   const resync = new Set<string>()
   const populateInstances = (occurrence: InstanceOccurrence): void => {
@@ -253,7 +286,7 @@ function materializeReader(
         linkInstanceSourceChildren(child, materialized, components)
         if (previous) {
           reconcileLiveComponentEdits(graph, materialized)
-          if (materialized.root.componentId) resync.add(materialized.root.componentId)
+          if (materialized.root.componentId) resync.add(materialized.root.id)
         }
         sources.set(child.sourceId, materialized.root.id)
       } else if (child.mainComponentId === null && child.properties.type !== 'SYMBOL')
@@ -267,7 +300,8 @@ function materializeReader(
       parent.childIds = [...ordered, ...parent.childIds.filter((id) => !ordered.includes(id))]
   }
   for (const page of pages) populateInstances(page)
-  for (const componentId of resync) graph.syncInstances(componentId)
+  for (const instanceId of resync) graph.syncInstance(instanceId)
+  if (resync.size > 0) restoreDerivedSizes(graph, derivedSizes)
   for (const node of graph.getAllNodes()) {
     if (existingNodeIds.has(node.id)) continue
     for (const field of [

packages/fig/src/instance-overrides/live-component-edits.ts

@@ -7,6 +7,38 @@ import {
 
 import type { MaterializedInstance } from './materialize-instance'
 
+/** The instances that own `target`, nearest first, starting with `target` when it is one. */
+function owningInstances(graph: SceneGraph, target: SceneNode): SceneNode[] {
+  const owners: SceneNode[] = []
+  let parent = target.parentId ? graph.getNode(target.parentId) : undefined
+  while (parent) {
+    if (parent.type === 'INSTANCE') owners.push(parent)
+    parent = parent.parentId ? graph.getNode(parent.parentId) : undefined
+  }
+  if (target.type === 'INSTANCE') owners.unshift(target)
+  return owners
+}
+
+/**
+ * The component layers `target` syncs from, one per owning instance: the layer the owner maps it
+ * to, or its own component.
+ */
+export function syncSourceLayers(graph: SceneGraph, target: SceneNode): SceneNode[] {
+  const sources: SceneNode[] = []
+  for (const owner of owningInstances(graph, target)) {
+    const mapped = getInstanceOverride(
+      owner.instanceOverrides,
+      owner.id,
+      target.id,
+      'sourceComponentId'
+    )
+    const sourceId = typeof mapped === 'string' ? mapped : target.componentId
+    const source = sourceId ? graph.getNode(sourceId) : undefined
+    if (source) sources.push(source)
+  }
+  return sources
+}
+
 /** Apply live component edits to new occurrences, never repaint or resync existing pages. */
 export function reconcileLiveComponentEdits(
   graph: SceneGraph,
@@ -14,13 +46,7 @@ export function reconcileLiveComponentEdits(
 ): void {
   graph.preserveSourceMetadataDuring(() => {
     for (const target of materialized.nodes.values()) {
-      const owners: SceneNode[] = []
-      let parent = target.parentId ? graph.getNode(target.parentId) : undefined
-      while (parent) {
-        if (parent.type === 'INSTANCE') owners.push(parent)
-        parent = parent.parentId ? graph.getNode(parent.parentId) : undefined
-      }
-      if (target.type === 'INSTANCE') owners.unshift(target)
+      const owners = owningInstances(graph, target)
       const protectedFields = new Set<string>()
       for (const owner of owners) {
         const fields =
@@ -30,16 +56,7 @@ export function reconcileLiveComponentEdits(
         for (const field of fields?.keys() ?? []) protectedFields.add(field)
       }
       const updates: Partial<SceneNode> = {}
-      for (const owner of owners) {
-        const mapped = getInstanceOverride(
-          owner.instanceOverrides,
-          owner.id,
-          target.id,
-          'sourceComponentId'
-        )
-        const sourceId = typeof mapped === 'string' ? mapped : target.componentId
-        const source = sourceId ? graph.getNode(sourceId) : undefined
-        if (!source) continue
+      for (const source of syncSourceLayers(graph, target)) {
         for (const field of INSTANCE_SYNC_FIELDS) {
           if (!source.source.editedFields.includes(field) || protectedFields.has(field)) continue
           Object.assign(updates, { [field]: structuredClone(source[field]) })

packages/scene-graph/src/index.ts

@@ -835,6 +835,10 @@ export class SceneGraph {
     Instances.syncInstances(this, componentId)
   }
 
+  syncInstance(instanceId: string): void {
+    Instances.syncInstance(this, instanceId)
+  }
+
   detachInstance(instanceId: string): void {
     Instances.detachInstance(this, instanceId)
   }

packages/scene-graph/src/instances.ts

@@ -125,6 +125,21 @@ export function swapInstanceComponent(
 const syncingComponentsByGraph = new WeakMap<SceneGraph, Set<string>>()
 
 export function syncInstances(graph: SceneGraph, componentId: string): void {
+  syncInstancesOf(graph, componentId, getInstances(graph, componentId))
+}
+
+/** Syncs one instance from its component, as `syncInstances` does for every instance. */
+export function syncInstance(graph: SceneGraph, instanceId: string): void {
+  const instance = graph.nodes.get(instanceId)
+  if (instance?.type !== 'INSTANCE' || !instance.componentId) return
+  syncInstancesOf(graph, instance.componentId, [instance])
+}
+
+function syncInstancesOf(
+  graph: SceneGraph,
+  componentId: string,
+  instances: Iterable<SceneNode>
+): void {
   const component = graph.nodes.get(componentId)
   if (component?.type !== 'COMPONENT') return
   let syncing = syncingComponentsByGraph.get(graph)
@@ -135,7 +150,7 @@ export function syncInstances(graph: SceneGraph, componentId: string): void {
   if (syncing.has(componentId)) return
   syncing.add(componentId)
   try {
-    for (const instance of getInstances(graph, componentId)) {
+    for (const instance of instances) {
       const enclosing = enclosingInstanceOverrideFields(graph, instance)
       enclosing.push(new Set(instance.instanceOverrides.self.keys()))
       const protectedField = bindingProtection(enclosing)

tests/engine/io/fig/import/instance-regressions.test.ts

@@ -57,14 +57,18 @@ describe('derived instance layout regressions', () => {
     expect(inputFrame?.width).toBeCloseTo(375.7498, 3)
     expect(inputFrame?.height).toBeCloseTo(39.3803, 3)
     expect(content).toMatchObject({ x: 0, y: 0 })
+    expect(content?.width).toBeCloseTo(374.8589782714844, 3)
     // Original Figma capture: gold-input-layout.json. The outer instance scales padding too.
     expect(firstBadge?.x).toBeCloseTo(7.126753330230713, 3)
     expect(firstBadge?.y).toBeCloseTo(5.345065116882324, 3)
     expect(firstBadge?.width).toBeCloseTo(85.3239, 3)
     expect(firstBadge?.height).toBeCloseTo(28.6901, 3)
     expect(firstBadgeContent).toMatchObject({ x: 0, y: 0 })
+    // Figma derived the placeholder at 97.507 × 39.3803, as tall as Content, in the file.
     expect(placeholderFrame?.x).toBeCloseTo(277.352, 3)
-    expect(placeholderFrame?.y).toBeCloseTo(0.0915584564, 3)
+    expect(placeholderFrame?.y).toBeCloseTo(0, 3)
+    expect(placeholderFrame?.width).toBeCloseTo(97.50701141357422, 3)
+    expect(placeholderFrame?.height).toBeCloseTo(39.3802604675293, 3)
     expect(placeholderText?.x).toBeCloseTo(14.2535, 3)
     expect(placeholderText?.y).toBeCloseTo(10.6901, 3)
   })