From b8b6f51b3a03cfd57c8a259767d85fdb2fe8ebc8 Mon Sep 17 00:00:00 2001 From: DrJKL Date: Thu, 14 May 2026 13:28:00 -0700 Subject: [PATCH] refactor(subgraph): address review nits on promotion + migration - proxyWidgetMigration: switch repairValue/migratePreview to exhaustive switches with a shared `assertUnreachablePlan(plan: never)` helper so adding a new Plan kind triggers a TS error at the dispatcher. - previewExposureStore: drop unused `moveExposure` action (and its tests); document deferred Object.freeze hardening at getExposures. - usePromotedPreviews: drop optional chaining on nodeOutputStore.getNode*ByExecutionId methods (always defined). - BaseWidget: drop the unused `_suppressPromotedOutline` parameter on `getOutlineColor` and the `suppressPromotedOutline` field on `DrawWidgetOptions`; remove all 14 call sites. - SubgraphNodeWidget.vue: switch to tuple-form `defineEmits<{ ... }>()`. - LGraph: rewrite the `proxyWidgetMigrationFlush` JSDoc to cite the real reason for late binding (the migration module's transitive workbench imports, not PreviewExposureStore); mark both `proxyWidgetMigrationFlush` and `autoExposePreviewNodes` `@internal`. - proxyWidgetMigration tests: update assertions to verify behavior (host inputs, exposures, quarantine entries) now that `flushProxyWidgetMigration` returns void. Amp-Thread-ID: https://ampcode.com/threads/T-019e2812-d683-710e-946f-9ddb9018ff5a Co-authored-by: Amp --- .../subgraph/SubgraphNodeWidget.vue | 4 +- src/composables/node/usePromotedPreviews.ts | 6 +- .../migration/proxyWidgetMigration.test.ts | 119 ++++++------------ .../migration/proxyWidgetMigration.ts | 74 ++++++----- src/core/graph/subgraph/promotedWidgetView.ts | 1 - src/core/graph/subgraph/promotionUtils.ts | 2 +- src/lib/litegraph/src/LGraph.ts | 16 ++- .../litegraph/src/subgraph/SubgraphNode.ts | 62 ++++----- src/lib/litegraph/src/widgets/BaseWidget.ts | 11 +- src/lib/litegraph/src/widgets/ButtonWidget.ts | 4 +- src/lib/litegraph/src/widgets/ChartWidget.ts | 2 +- .../litegraph/src/widgets/FileUploadWidget.ts | 2 +- .../litegraph/src/widgets/GalleriaWidget.ts | 2 +- .../src/widgets/ImageCompareWidget.ts | 2 +- src/lib/litegraph/src/widgets/KnobWidget.ts | 6 +- .../litegraph/src/widgets/MarkdownWidget.ts | 2 +- .../src/widgets/MultiSelectWidget.ts | 2 +- .../src/widgets/SelectButtonWidget.ts | 2 +- src/lib/litegraph/src/widgets/SliderWidget.ts | 4 +- src/stores/previewExposureStore.test.ts | 39 ------ src/stores/previewExposureStore.ts | 31 +---- 21 files changed, 138 insertions(+), 255 deletions(-) diff --git a/src/components/rightSidePanel/subgraph/SubgraphNodeWidget.vue b/src/components/rightSidePanel/subgraph/SubgraphNodeWidget.vue index d8783129f9..5ccc1dae54 100644 --- a/src/components/rightSidePanel/subgraph/SubgraphNodeWidget.vue +++ b/src/components/rightSidePanel/subgraph/SubgraphNodeWidget.vue @@ -20,9 +20,7 @@ const { isShown?: boolean class?: ClassValue }>() -defineEmits<{ - (e: 'toggleVisibility'): void -}>() +defineEmits<{ toggleVisibility: [] }>() const icon = computed(() => isPhysical diff --git a/src/composables/node/usePromotedPreviews.ts b/src/composables/node/usePromotedPreviews.ts index 4473f679a6..98aaa7f839 100644 --- a/src/composables/node/usePromotedPreviews.ts +++ b/src/composables/node/usePromotedPreviews.ts @@ -64,9 +64,9 @@ export function usePromotedPreviews( const reactiveOutputs = nodeOutputStore.nodeOutputs[locatorId] const reactivePreviews = nodeOutputStore.nodePreviewImages[locatorId] const reactiveExecutionOutputs = - nodeOutputStore.getNodeOutputByExecutionId?.(leafExecutionId) + nodeOutputStore.getNodeOutputByExecutionId(leafExecutionId) const reactiveExecutionPreviews = - nodeOutputStore.getNodePreviewImagesByExecutionId?.(leafExecutionId) + nodeOutputStore.getNodePreviewImagesByExecutionId(leafExecutionId) const hasAnySource = reactiveOutputs?.images?.length || reactivePreviews?.length || @@ -74,7 +74,7 @@ export function usePromotedPreviews( reactiveExecutionPreviews?.length if (!hasAnySource) return undefined return ( - nodeOutputStore.getNodeImageUrlsByExecutionId?.( + nodeOutputStore.getNodeImageUrlsByExecutionId( leafExecutionId, interiorNode ) ?? nodeOutputStore.getNodeImageUrls(interiorNode) diff --git a/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts b/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts index e492dfdf93..26352e8b7f 100644 --- a/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts +++ b/src/core/graph/subgraph/migration/proxyWidgetMigration.test.ts @@ -125,14 +125,8 @@ describe('flushProxyWidgetMigration', () => { it('returns an empty result when no proxyWidgets are present', () => { const host = buildHost() - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toEqual({ - repaired: 0, - primitiveRepaired: 0, - previewMigrated: 0, - quarantined: 0 - }) expect(host.properties.proxyWidgets).toBeUndefined() }) @@ -140,14 +134,8 @@ describe('flushProxyWidgetMigration', () => { const host = buildHost() host.properties.proxyWidgets = '{not json}' - const result = flushProxyWidgetMigration({ hostNode: host }) - - expect(result).toEqual({ - repaired: 0, - primitiveRepaired: 0, - previewMigrated: 0, - quarantined: 0 - }) + expect(() => flushProxyWidgetMigration({ hostNode: host })).not.toThrow() + expect(host.properties.proxyWidgetErrorQuarantine).toBeUndefined() }) }) @@ -166,12 +154,11 @@ describe('flushProxyWidgetMigration', () => { }) host.properties.proxyWidgets = [[String(inner.id), 'seed']] - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: [99] }) - expect(result).toMatchObject({ repaired: 1, quarantined: 0 }) expect(handle.getValue()).toBe(99) expect(host.properties.proxyWidgets).toBeUndefined() }) @@ -190,12 +177,11 @@ describe('flushProxyWidgetMigration', () => { subgraph.inputNode.slots[0].connect(inner.inputs[0], inner) host.properties.proxyWidgets = [[String(inner.id), 'seed']] - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: [99] }) - expect(result).toMatchObject({ repaired: 1, quarantined: 0 }) expect(host.widgets[0].value).toBe(99) const innerWidget = inner.widgets!.find((w) => w.name === 'seed')! expect(innerWidget.value).toBe(0) @@ -216,12 +202,11 @@ describe('flushProxyWidgetMigration', () => { host.properties.proxyWidgets = [[String(inner.id), 'seed']] const sparse: unknown[] = [] - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: sparse }) - expect(result).toMatchObject({ repaired: 1, quarantined: 0 }) expect(handle.getValue()).toBe(7) }) @@ -246,12 +231,11 @@ describe('flushProxyWidgetMigration', () => { }) host.properties.proxyWidgets = [[String(inner.id), 'seed']] - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: [99] }) - expect(result).toMatchObject({ repaired: 0, quarantined: 1 }) expect(a.getValue()).toBe(1) expect(b.getValue()).toBe(2) expect(readHostQuarantine(host)).toEqual([ @@ -272,9 +256,8 @@ describe('flushProxyWidgetMigration', () => { const inputCountBefore = host.subgraph.inputs.length host.properties.proxyWidgets = [[String(inner.id), 'seed']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ repaired: 1, quarantined: 0 }) expect(host.subgraph.inputs).toHaveLength(inputCountBefore + 1) const created = host.subgraph.inputs.at(-1) expect(created?._widget).toBeDefined() @@ -302,9 +285,8 @@ describe('flushProxyWidgetMigration', () => { }) host.properties.proxyWidgets = [[String(inner.id), 'text', '2']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ repaired: 1, quarantined: 0 }) const created = host.subgraph.inputs.at(-1) expect(created?._widget).toBeDefined() // The created SubgraphInput connects to inner's "text_1" slot (the @@ -323,9 +305,8 @@ describe('flushProxyWidgetMigration', () => { const inputCountBefore = host.subgraph.inputs.length host.properties.proxyWidgets = [[String(inner.id), 'seed']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ repaired: 0, quarantined: 1 }) expect(host.subgraph.inputs).toHaveLength(inputCountBefore) expect(readHostQuarantine(host)).toEqual([ expect.objectContaining({ @@ -345,13 +326,8 @@ describe('flushProxyWidgetMigration', () => { const inputCountBefore = host.subgraph.inputs.length host.properties.proxyWidgets = [[String(primitive.id), 'value']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ - primitiveRepaired: 1, - repaired: 0, - quarantined: 0 - }) expect(host.subgraph.inputs).toHaveLength(inputCountBefore + 1) for (const target of targets) { const slot = target.inputs[0] @@ -371,9 +347,8 @@ describe('flushProxyWidgetMigration', () => { [String(primitive.id), 'value'], [String(primitive.id), 'value'] ] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ primitiveRepaired: 1, quarantined: 0 }) for (const target of targets) { const slot = target.inputs[0] const link = host.subgraph.links.get(slot.link!) @@ -389,11 +364,10 @@ describe('flushProxyWidgetMigration', () => { }) host.properties.proxyWidgets = [[String(primitive.id), 'value']] - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: [123] }) - expect(result.primitiveRepaired).toBe(1) // Host value lands on the host's input mirror (a `PromotedWidgetView`), // not on the shared interior consumer widget. Verifying the host side @@ -410,9 +384,8 @@ describe('flushProxyWidgetMigration', () => { }) host.properties.proxyWidgets = [[String(primitive.id), 'value']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result.primitiveRepaired).toBe(1) // With no host value supplied, the host is seeded per-instance from // the primitive's widget value — never by mutating the shared interior. const hostInput = host.inputs.at(-1) @@ -427,9 +400,8 @@ describe('flushProxyWidgetMigration', () => { host.subgraph.add(primitive) host.properties.proxyWidgets = [[String(primitive.id), 'value']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ primitiveRepaired: 0, quarantined: 1 }) expect(readHostQuarantine(host)).toEqual([ expect.objectContaining({ originalEntry: [String(primitive.id), 'value'], @@ -447,9 +419,8 @@ describe('flushProxyWidgetMigration', () => { const inputCountBefore = host.subgraph.inputs.length host.properties.proxyWidgets = [[String(primitive.id), 'value']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ primitiveRepaired: 0, quarantined: 1 }) expect(host.subgraph.inputs).toHaveLength(inputCountBefore) expect(readHostQuarantine(host)).toEqual([ expect.objectContaining({ @@ -471,9 +442,8 @@ describe('flushProxyWidgetMigration', () => { ] host.properties.proxyWidgets = [[String(primitive.id), 'value']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ primitiveRepaired: 0, quarantined: 1 }) expect(readHostQuarantine(host)).toEqual([ expect.objectContaining({ originalEntry: [String(primitive.id), 'value'], @@ -511,21 +481,20 @@ describe('flushProxyWidgetMigration', () => { hostA.properties.proxyWidgets = [[String(primitive.id), 'value']] hostB.properties.proxyWidgets = [[String(primitive.id), 'value']] - const resultA = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: hostA, hostWidgetValues: [11] }) - const resultB = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: hostB, hostWidgetValues: [22] }) - expect(resultA).toMatchObject({ primitiveRepaired: 1, quarantined: 0 }) // Host B's classify recognises the bypass marker on the primitive and - // takes the `alreadyLinked` path, so it counts as `repaired` not - // `primitiveRepaired`. Either way, no quarantine. - expect(resultB).toMatchObject({ quarantined: 0 }) - expect(resultB.repaired + resultB.primitiveRepaired).toBe(1) + // takes the `alreadyLinked` path. Either way, no quarantine, and each + // host gets an independent value. + expect(hostA.properties.proxyWidgetErrorQuarantine).toBeUndefined() + expect(hostB.properties.proxyWidgetErrorQuarantine).toBeUndefined() const widgetA = hostA.inputs.at(-1)?._widget const widgetB = hostB.inputs.at(-1)?._widget @@ -544,9 +513,8 @@ describe('flushProxyWidgetMigration', () => { host.properties.proxyWidgets = [ [String(inner.id), '$$canvas-image-preview'] ] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ previewMigrated: 1, quarantined: 0 }) const exposures = usePreviewExposureStore().getExposures( host.rootGraph.id, String(host.id) @@ -565,9 +533,8 @@ describe('flushProxyWidgetMigration', () => { }) host.properties.proxyWidgets = [[String(inner.id), 'videopreview']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ previewMigrated: 1, quarantined: 0 }) const exposures = usePreviewExposureStore().getExposures( host.rootGraph.id, String(host.id) @@ -599,9 +566,8 @@ describe('flushProxyWidgetMigration', () => { host.properties.proxyWidgets = [ [String(innerB.id), '$$canvas-image-preview'] ] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ previewMigrated: 1, quarantined: 0 }) const exposures = store.getExposures(host.rootGraph.id, locator) expect(exposures).toHaveLength(2) const newExposure = exposures.find( @@ -626,9 +592,8 @@ describe('flushProxyWidgetMigration', () => { host.properties.proxyWidgets = [ [String(inner.id), '$$canvas-image-preview'] ] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ previewMigrated: 1, quarantined: 0 }) expect(store.getExposures(host.rootGraph.id, locator)).toHaveLength(1) }) }) @@ -638,9 +603,8 @@ describe('flushProxyWidgetMigration', () => { const host = buildHost() host.properties.proxyWidgets = [['9999', 'seed']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ quarantined: 1 }) expect(readHostQuarantine(host)).toEqual([ { originalEntry: ['9999', 'seed'], @@ -655,9 +619,8 @@ describe('flushProxyWidgetMigration', () => { const inner = addInnerNode(host, 'Inner') host.properties.proxyWidgets = [[String(inner.id), 'nonexistent']] - const result = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(result).toMatchObject({ quarantined: 1 }) expect(readHostQuarantine(host)).toEqual([ expect.objectContaining({ originalEntry: [String(inner.id), 'nonexistent'], @@ -740,21 +703,17 @@ describe('flushProxyWidgetMigration', () => { [String(inner.id), '$$canvas-image-preview'] ] - const first = flushProxyWidgetMigration({ hostNode: host }) - expect(first.previewMigrated).toBe(1) + flushProxyWidgetMigration({ hostNode: host }) const exposuresAfterFirst = usePreviewExposureStore() .getExposures(host.rootGraph.id, String(host.id)) .map((e) => ({ ...e })) + expect(exposuresAfterFirst).toHaveLength(1) - const second = flushProxyWidgetMigration({ hostNode: host }) + flushProxyWidgetMigration({ hostNode: host }) - expect(second).toEqual({ - repaired: 0, - primitiveRepaired: 0, - previewMigrated: 0, - quarantined: 0 - }) + // Re-running the flush is idempotent: no new exposures, no quarantine. + expect(host.properties.proxyWidgetErrorQuarantine).toBeUndefined() expect( usePreviewExposureStore().getExposures( host.rootGraph.id, @@ -779,16 +738,11 @@ describe('flushProxyWidgetMigration', () => { [String(inner.id), 'seed'], [String(inner.id), '$$canvas-image-preview'] ] - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: [99] }) - expect(result).toMatchObject({ - repaired: 1, - previewMigrated: 1, - quarantined: 0 - }) expect(host.subgraph.inputs).toHaveLength(subgraphInputCountBefore + 1) expect(host.subgraph.inputs.find((i) => i.name === 'seed')).toBeDefined() const exposures = usePreviewExposureStore().getExposures( @@ -816,12 +770,11 @@ describe('flushProxyWidgetMigration', () => { ] const sparse: unknown[] = [] sparse[1] = 'second-value' - const result = flushProxyWidgetMigration({ + flushProxyWidgetMigration({ hostNode: host, hostWidgetValues: sparse }) - expect(result).toMatchObject({ repaired: 2, quarantined: 0 }) expect(host.subgraph.inputs.find((i) => i.name === 'a')).toBeDefined() expect(host.subgraph.inputs.find((i) => i.name === 'b')).toBeDefined() }) diff --git a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts index 7c7b5ed0b3..aba1353a1d 100644 --- a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts +++ b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts @@ -90,20 +90,6 @@ interface FlushArgs { hostWidgetValues?: readonly unknown[] } -interface FlushResult { - repaired: number - primitiveRepaired: number - previewMigrated: number - quarantined: number -} - -const EMPTY_RESULT: FlushResult = Object.freeze({ - repaired: 0, - primitiveRepaired: 0, - previewMigrated: 0, - quarantined: 0 -}) - interface PrimitiveBypassTargetRef { targetNodeId: NodeId targetSlot: number @@ -140,11 +126,11 @@ const QUARANTINE_VERSION = 1 */ const PROXY_BYPASS_MARKER_PROPERTY = 'proxyBypassedToSubgraphInput' -export function flushProxyWidgetMigration(args: FlushArgs): FlushResult { +export function flushProxyWidgetMigration(args: FlushArgs): void { const { hostNode, hostWidgetValues } = args const tuples = parseProxyWidgets(hostNode.properties.proxyWidgets) - if (tuples.length === 0) return EMPTY_RESULT + if (tuples.length === 0) return const cohort: LegacyProxyEntrySource[] = tuples.map( ([sourceNodeId, sourceWidgetName, disambiguator]) => @@ -167,7 +153,6 @@ export function flushProxyWidgetMigration(args: FlushArgs): FlushResult { }) const previewStore = usePreviewExposureStore() - const result: FlushResult = { ...EMPTY_RESULT } const quarantineToAppend: ProxyWidgetErrorQuarantineEntry[] = [] const primitiveCohorts = new Map() @@ -182,14 +167,12 @@ export function flushProxyWidgetMigration(args: FlushArgs): FlushResult { case 'alreadyLinked': case 'createSubgraphInput': { const r = repairValue(hostNode, entry) - if (r.ok) result.repaired += 1 - else quarantineToAppend.push(quarantineFor(entry, r.reason)) + if (!r.ok) quarantineToAppend.push(quarantineFor(entry, r.reason)) break } case 'previewExposure': { const r = migratePreview(hostNode, entry, previewStore) - if (r.ok) result.previewMigrated += 1 - else quarantineToAppend.push(quarantineFor(entry, r.reason)) + if (!r.ok) quarantineToAppend.push(quarantineFor(entry, r.reason)) break } case 'quarantine': @@ -200,18 +183,15 @@ export function flushProxyWidgetMigration(args: FlushArgs): FlushResult { for (const c of primitiveCohorts.values()) { const r = repairPrimitive(hostNode, c) - if (r.ok) result.primitiveRepaired += 1 - else for (const e of c) quarantineToAppend.push(quarantineFor(e, r.reason)) + if (!r.ok) + for (const e of c) quarantineToAppend.push(quarantineFor(e, r.reason)) } if (quarantineToAppend.length > 0) { appendQuarantine(hostNode, quarantineToAppend) - result.quarantined = quarantineToAppend.length } delete hostNode.properties.proxyWidgets - - return result } function pickHostValue( @@ -411,17 +391,25 @@ function repairValue( hostNode: SubgraphNode, entry: PendingEntry ): RepairValueResult { - if (entry.plan.kind === 'alreadyLinked') { - return repairAlreadyLinked(hostNode, entry, entry.plan.subgraphInputName) + const { plan } = entry + switch (plan.kind) { + case 'alreadyLinked': + return repairAlreadyLinked(hostNode, entry, plan.subgraphInputName) + case 'createSubgraphInput': + return repairCreateSubgraphInput(hostNode, entry, plan.sourceWidgetName) + case 'primitiveBypass': + case 'previewExposure': + case 'quarantine': + throw new Error(`repairValue: unexpected plan kind ${plan.kind}`) + default: + return assertUnreachablePlan(plan) } - if (entry.plan.kind === 'createSubgraphInput') { - return repairCreateSubgraphInput( - hostNode, - entry, - entry.plan.sourceWidgetName - ) - } - throw new Error(`repairValue: invalid plan kind ${entry.plan.kind}`) +} + +function assertUnreachablePlan(plan: never): never { + throw new Error( + `Unexpected plan kind: ${(plan as { kind: string } | undefined)?.kind}` + ) } function repairAlreadyLinked( @@ -726,9 +714,17 @@ function migratePreview( entry: PendingEntry, store: ReturnType ): MigratePreviewResult { - const plan = entry.plan - if (plan.kind !== 'previewExposure') { - throw new Error(`migratePreview: invalid plan kind ${plan.kind}`) + const { plan } = entry + switch (plan.kind) { + case 'previewExposure': + break + case 'alreadyLinked': + case 'createSubgraphInput': + case 'primitiveBypass': + case 'quarantine': + throw new Error(`migratePreview: unexpected plan kind ${plan.kind}`) + default: + assertUnreachablePlan(plan) } const sourceNode = hostNode.subgraph.getNodeById( diff --git a/src/core/graph/subgraph/promotedWidgetView.ts b/src/core/graph/subgraph/promotedWidgetView.ts index 2a634604e4..3c8c20929c 100644 --- a/src/core/graph/subgraph/promotedWidgetView.ts +++ b/src/core/graph/subgraph/promotedWidgetView.ts @@ -382,7 +382,6 @@ class PromotedWidgetView implements IPromotedWidgetView { projected.drawWidget(ctx, { width: widgetWidth, showText: !lowQuality, - suppressPromotedOutline: true, previewImages: resolved.node.imgs }) } finally { diff --git a/src/core/graph/subgraph/promotionUtils.ts b/src/core/graph/subgraph/promotionUtils.ts index 3d599fe8ea..bcdd41fb4c 100644 --- a/src/core/graph/subgraph/promotionUtils.ts +++ b/src/core/graph/subgraph/promotionUtils.ts @@ -248,7 +248,7 @@ function toPromotionSource( function refreshPromotedWidgetRendering(parents: SubgraphNode[]): void { for (const parent of parents) { parent.computeSize(parent.size) - parent.setDirtyCanvas?.(true, true) + parent.setDirtyCanvas(true, true) } useCanvasStore().canvas?.setDirty(true, true) } diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index 662baeffab..ec4b9c9b6f 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -183,9 +183,16 @@ export class LGraph static STATUS_RUNNING = 2 /** - * Late-bound migration hook. Set once during app init from the wiring layer - * to avoid a circular dependency through PreviewExposureStore. Left undefined - * in tests that exercise `configure()` without the migration pipeline. + * Late-bound migration hook. Set once during app init from the wiring layer. + * The migration code lives in `@/core/graph/subgraph/migration`, which + * transitively imports more workbench surface than the litegraph core is + * willing to depend on; binding it here lazily keeps LGraph free of those + * imports while still letting `configure()` invoke the flush. Left + * undefined in tests that exercise `configure()` without the migration + * pipeline. + * + * @internal Wiring hook for the frontend; not part of the public litegraph + * API. Custom nodes and extensions must not read or assign this. */ static proxyWidgetMigrationFlush?: ( hostNode: SubgraphNode, @@ -198,6 +205,9 @@ export class LGraph * Used after configure (workflow load and paste) so older clipboard / * workflow data without `properties.previewExposures` still surfaces a * preview on the host SubgraphNode. Idempotent. + * + * @internal Wiring hook for the frontend; not part of the public litegraph + * API. Custom nodes and extensions must not read or assign this. */ static autoExposePreviewNodes?: (hostNode: SubgraphNode) => void diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.ts index 4b6b28878d..06bac215f4 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.ts @@ -271,11 +271,6 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { this._cacheVersion++ } - private _mutateInputs(fn: () => void): void { - fn() - this.invalidatePromotedViews() - } - private _makePromotionViewKey( inputKey: string, sourceNodeId: string, @@ -349,12 +344,10 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { ) return } - let input!: INodeInputSlot & Partial - this._mutateInputs(() => { - input = this.addInput(name, type, { - _subgraphSlot: subgraphInput - }) + const input = this.addInput(name, type, { + _subgraphSlot: subgraphInput }) + this.invalidatePromotedViews() this._addSubgraphInputListeners(subgraphInput, input) }, @@ -367,7 +360,8 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { const widget = e.detail.input._widget if (widget) this.ensureWidgetRemoved(widget) - this._mutateInputs(() => this.removeInput(e.detail.index)) + this.removeInput(e.detail.index) + this.invalidatePromotedViews() this.setDirtyCanvas(true, true) }, { signal } @@ -597,29 +591,28 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { } } - this._mutateInputs(() => { - this.inputs.length = 0 - this.inputs.push( - ...this.subgraph.inputNode.slots.map((slot) => - Object.assign( - new NodeInputSlot( - { - name: slot.name, - localized_name: slot.localized_name, - label: slot.label, - shape: this.getSlotShape(slot), - type: slot.type, - link: null - }, - this - ), + this.inputs.length = 0 + this.inputs.push( + ...this.subgraph.inputNode.slots.map((slot) => + Object.assign( + new NodeInputSlot( { - _subgraphSlot: slot - } - ) + name: slot.name, + localized_name: slot.localized_name, + label: slot.label, + shape: this.getSlotShape(slot), + type: slot.type, + link: null + }, + this + ), + { + _subgraphSlot: slot + } ) ) - }) + ) + this.invalidatePromotedViews() this.outputs.length = 0 this.outputs.push( @@ -669,10 +662,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { // Prune inputs that don't map to any subgraph slot definition. // This prevents stale/duplicate serialized inputs from persisting (#9977). - this._mutateInputs(() => { - this.inputs = this.inputs.filter((input) => input._subgraphSlot) - }) - + this.inputs = this.inputs.filter((input) => input._subgraphSlot) this._promotedViewManager.clear() this.invalidatePromotedViews() @@ -693,8 +683,6 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { this._addSubgraphInputListeners(subgraphInput, input) this._resolveInputWidget(subgraphInput, input) } - - this.invalidatePromotedViews() } /** diff --git a/src/lib/litegraph/src/widgets/BaseWidget.ts b/src/lib/litegraph/src/widgets/BaseWidget.ts index 2ac26fc871..8199ca5267 100644 --- a/src/lib/litegraph/src/widgets/BaseWidget.ts +++ b/src/lib/litegraph/src/widgets/BaseWidget.ts @@ -27,7 +27,6 @@ export interface DrawWidgetOptions { width: number /** Synonym for "low quality". */ showText?: boolean - suppressPromotedOutline?: boolean /** Transient image source for preview widgets rendered on behalf of another node (e.g. subgraph promotion). */ previewImages?: HTMLImageElement[] } @@ -223,7 +222,7 @@ export abstract class BaseWidget } } - getOutlineColor(_suppressPromotedOutline = false) { + getOutlineColor() { return this.advanced ? LiteGraph.WIDGET_ADVANCED_OUTLINE_COLOR : LiteGraph.WIDGET_OUTLINE_COLOR @@ -287,13 +286,13 @@ export abstract class BaseWidget */ protected drawWidgetShape( ctx: CanvasRenderingContext2D, - { width, showText, suppressPromotedOutline }: DrawWidgetOptions + { width, showText }: DrawWidgetOptions ): void { const { height, y } = this const { margin } = BaseWidget ctx.textAlign = 'left' - ctx.strokeStyle = this.getOutlineColor(suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.fillStyle = this.background_color ctx.beginPath() @@ -314,7 +313,7 @@ export abstract class BaseWidget */ protected drawVueOnlyWarning( ctx: CanvasRenderingContext2D, - { width, suppressPromotedOutline }: DrawWidgetOptions, + { width }: DrawWidgetOptions, label: string ): void { const { y, height } = this @@ -324,7 +323,7 @@ export abstract class BaseWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/ButtonWidget.ts b/src/lib/litegraph/src/widgets/ButtonWidget.ts index fe9659c231..7f084332cc 100644 --- a/src/lib/litegraph/src/widgets/ButtonWidget.ts +++ b/src/lib/litegraph/src/widgets/ButtonWidget.ts @@ -23,7 +23,7 @@ export class ButtonWidget */ override drawWidget( ctx: CanvasRenderingContext2D, - { width, showText = true, suppressPromotedOutline }: DrawWidgetOptions + { width, showText = true }: DrawWidgetOptions ) { // Store original context attributes const { fillStyle, strokeStyle, textAlign } = ctx @@ -41,7 +41,7 @@ export class ButtonWidget // Draw button outline if not disabled if (showText && !this.computedDisabled) { - ctx.strokeStyle = this.getOutlineColor(suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(margin, y, width - margin * 2, height) } diff --git a/src/lib/litegraph/src/widgets/ChartWidget.ts b/src/lib/litegraph/src/widgets/ChartWidget.ts index 94be0183a5..83961a2d2a 100644 --- a/src/lib/litegraph/src/widgets/ChartWidget.ts +++ b/src/lib/litegraph/src/widgets/ChartWidget.ts @@ -23,7 +23,7 @@ export class ChartWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/FileUploadWidget.ts b/src/lib/litegraph/src/widgets/FileUploadWidget.ts index f9d41c3d84..694d8dbad3 100644 --- a/src/lib/litegraph/src/widgets/FileUploadWidget.ts +++ b/src/lib/litegraph/src/widgets/FileUploadWidget.ts @@ -23,7 +23,7 @@ export class FileUploadWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/GalleriaWidget.ts b/src/lib/litegraph/src/widgets/GalleriaWidget.ts index 5c596ff978..2ede905dca 100644 --- a/src/lib/litegraph/src/widgets/GalleriaWidget.ts +++ b/src/lib/litegraph/src/widgets/GalleriaWidget.ts @@ -23,7 +23,7 @@ export class GalleriaWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/ImageCompareWidget.ts b/src/lib/litegraph/src/widgets/ImageCompareWidget.ts index 435ed6b388..a3bb3d1f4f 100644 --- a/src/lib/litegraph/src/widgets/ImageCompareWidget.ts +++ b/src/lib/litegraph/src/widgets/ImageCompareWidget.ts @@ -23,7 +23,7 @@ export class ImageCompareWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/KnobWidget.ts b/src/lib/litegraph/src/widgets/KnobWidget.ts index 600d802f12..7b293c60e3 100644 --- a/src/lib/litegraph/src/widgets/KnobWidget.ts +++ b/src/lib/litegraph/src/widgets/KnobWidget.ts @@ -33,7 +33,7 @@ export class KnobWidget extends BaseWidget implements IKnobWidget { drawWidget( ctx: CanvasRenderingContext2D, - { width, showText = true, suppressPromotedOutline }: DrawWidgetOptions + { width, showText = true }: DrawWidgetOptions ): void { // Store original context attributes const { fillStyle, strokeStyle, textAlign } = ctx @@ -145,10 +145,10 @@ export class KnobWidget extends BaseWidget implements IKnobWidget { // Draw outline if not disabled if (showText && !this.computedDisabled) { - ctx.strokeStyle = this.getOutlineColor(suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() // Draw value ctx.beginPath() - ctx.strokeStyle = this.getOutlineColor(suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.arc( arc_center.x, arc_center.y, diff --git a/src/lib/litegraph/src/widgets/MarkdownWidget.ts b/src/lib/litegraph/src/widgets/MarkdownWidget.ts index 5d742fc3be..e2847a06be 100644 --- a/src/lib/litegraph/src/widgets/MarkdownWidget.ts +++ b/src/lib/litegraph/src/widgets/MarkdownWidget.ts @@ -23,7 +23,7 @@ export class MarkdownWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/MultiSelectWidget.ts b/src/lib/litegraph/src/widgets/MultiSelectWidget.ts index ace3a71acc..961696a230 100644 --- a/src/lib/litegraph/src/widgets/MultiSelectWidget.ts +++ b/src/lib/litegraph/src/widgets/MultiSelectWidget.ts @@ -23,7 +23,7 @@ export class MultiSelectWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/SelectButtonWidget.ts b/src/lib/litegraph/src/widgets/SelectButtonWidget.ts index 4fff959017..0236f4207d 100644 --- a/src/lib/litegraph/src/widgets/SelectButtonWidget.ts +++ b/src/lib/litegraph/src/widgets/SelectButtonWidget.ts @@ -23,7 +23,7 @@ export class SelectButtonWidget ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) - ctx.strokeStyle = this.getOutlineColor(options.suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color diff --git a/src/lib/litegraph/src/widgets/SliderWidget.ts b/src/lib/litegraph/src/widgets/SliderWidget.ts index 434743ac47..ce7146f547 100644 --- a/src/lib/litegraph/src/widgets/SliderWidget.ts +++ b/src/lib/litegraph/src/widgets/SliderWidget.ts @@ -20,7 +20,7 @@ export class SliderWidget */ override drawWidget( ctx: CanvasRenderingContext2D, - { width, showText = true, suppressPromotedOutline }: DrawWidgetOptions + { width, showText = true }: DrawWidgetOptions ) { // Store original context attributes const { fillStyle, strokeStyle, textAlign } = ctx @@ -43,7 +43,7 @@ export class SliderWidget // Draw outline if not disabled if (showText && !this.computedDisabled) { - ctx.strokeStyle = this.getOutlineColor(suppressPromotedOutline) + ctx.strokeStyle = this.getOutlineColor() ctx.strokeRect(margin, y, width - margin * 2, height) } diff --git a/src/stores/previewExposureStore.test.ts b/src/stores/previewExposureStore.test.ts index 4cd41415ad..d341c17d24 100644 --- a/src/stores/previewExposureStore.test.ts +++ b/src/stores/previewExposureStore.test.ts @@ -122,45 +122,6 @@ describe(usePreviewExposureStore, () => { }) }) - describe('moveExposure', () => { - beforeEach(() => { - store.setExposures(rootGraphA, hostA, [ - { name: 'a', sourceNodeId: '1', sourcePreviewName: 'a' }, - { name: 'b', sourceNodeId: '2', sourcePreviewName: 'b' }, - { name: 'c', sourceNodeId: '3', sourcePreviewName: 'c' } - ]) - }) - - it('reorders entries from -> to', () => { - store.moveExposure(rootGraphA, hostA, 0, 2) - - expect(store.getExposures(rootGraphA, hostA).map((e) => e.name)).toEqual([ - 'b', - 'c', - 'a' - ]) - }) - - it('is a no-op for equal indices', () => { - store.moveExposure(rootGraphA, hostA, 1, 1) - expect(store.getExposures(rootGraphA, hostA).map((e) => e.name)).toEqual([ - 'a', - 'b', - 'c' - ]) - }) - - it('is a no-op for out-of-bounds indices', () => { - store.moveExposure(rootGraphA, hostA, -1, 2) - store.moveExposure(rootGraphA, hostA, 0, 5) - expect(store.getExposures(rootGraphA, hostA).map((e) => e.name)).toEqual([ - 'a', - 'b', - 'c' - ]) - }) - }) - describe('clearGraph', () => { it('removes all hosts under the rootGraphId without affecting others', () => { store.addExposure(rootGraphA, hostA, { diff --git a/src/stores/previewExposureStore.ts b/src/stores/previewExposureStore.ts index 7e1a534c36..5138279533 100644 --- a/src/stores/previewExposureStore.ts +++ b/src/stores/previewExposureStore.ts @@ -42,6 +42,11 @@ export const usePreviewExposureStore = defineStore('previewExposure', () => { return exposures.value.get(rootGraphId)?.get(hostNodeLocator) } + /** + * Returns the live internal array (typed `readonly` to discourage mutation). + * Defensive `Object.freeze` at the four insert sites would close this if + * a caller ever casts away the readonly modifier; deferred until needed. + */ function getExposures( rootGraphId: UUID, hostNodeLocator: string @@ -93,31 +98,6 @@ export const usePreviewExposureStore = defineStore('previewExposure', () => { setExposures(rootGraphId, hostNodeLocator, next) } - function moveExposure( - rootGraphId: UUID, - hostNodeLocator: string, - fromIndex: number, - toIndex: number - ): void { - const hosts = exposures.value.get(rootGraphId) - const current = hosts?.get(hostNodeLocator) - if (!hosts || !current?.length) return - - if ( - fromIndex < 0 || - fromIndex >= current.length || - toIndex < 0 || - toIndex >= current.length || - fromIndex === toIndex - ) - return - - const next = [...current] - const [entry] = next.splice(fromIndex, 1) - next.splice(toIndex, 0, entry) - hosts.set(hostNodeLocator, next) - } - function clearGraph(rootGraphId: UUID): void { exposures.value.delete(rootGraphId) } @@ -148,7 +128,6 @@ export const usePreviewExposureStore = defineStore('previewExposure', () => { setExposures, addExposure, removeExposure, - moveExposure, clearGraph, resolveChain }