From 837cc536933249aa3b916166b9bd84f049ffeb20 Mon Sep 17 00:00:00 2001 From: DrJKL Date: Thu, 14 May 2026 13:58:21 -0700 Subject: [PATCH] refactor(subgraph): consolidate type guards and tighten review nits - Extract canonical isWidgetValue helper to types/widgets.ts and dedupe copies in promotedWidgetView, promotionUtils, and SubgraphNode - Replace cast in proxyWidgetMigration.pickHostValue with isWidgetValue narrowing; treat invalid host payloads as holes - Replace 'getSlotFromWidget' duck-type guard in promoteWidget with instanceof LGraphNode narrowing - Add DEV-only console.warn in SubgraphNode.serialize() when host widgets contain non-promoted entries (per ADR 0009) - Extract LGraphTriggerActions tuple as single source of truth for the runtime Set and the LGraphTriggerAction type union - Replace in-place tuple slot mutation in appModeStore.updateInputConfig with splice replacing the whole entry - Drop dead setter on activeWidgets computed in SubgraphEditor.vue Amp-Thread-ID: https://ampcode.com/threads/T-019e2812-d683-710e-946f-9ddb9018ff5a Co-authored-by: Amp --- .../subgraph/SubgraphEditor.vue | 14 ++---- .../migration/proxyWidgetMigration.ts | 8 +++- src/core/graph/subgraph/promotedWidgetView.ts | 9 +--- src/core/graph/subgraph/promotionUtils.ts | 48 +++++++------------ src/lib/litegraph/src/LGraph.ts | 12 ++--- .../litegraph/src/subgraph/SubgraphNode.ts | 32 ++++++++----- src/lib/litegraph/src/types/graphTriggers.ts | 15 +++++- src/lib/litegraph/src/types/widgets.ts | 15 ++++++ src/stores/appModeStore.ts | 11 +++-- 9 files changed, 91 insertions(+), 73 deletions(-) diff --git a/src/components/rightSidePanel/subgraph/SubgraphEditor.vue b/src/components/rightSidePanel/subgraph/SubgraphEditor.vue index 65ab2fe706..2e193c64fa 100644 --- a/src/components/rightSidePanel/subgraph/SubgraphEditor.vue +++ b/src/components/rightSidePanel/subgraph/SubgraphEditor.vue @@ -41,16 +41,10 @@ const activeNode = computed(() => { return undefined }) -const activeWidgets = computed({ - get() { - const node = activeNode.value - if (!node) return [] - - return [...getActivePromotedWidgets(node), ...getActivePreviewWidgets(node)] - }, - set(value: WidgetItem[]) { - updateActiveWidgets(value, activeWidgets.value) - } +const activeWidgets = computed(() => { + const node = activeNode.value + if (!node) return [] + return [...getActivePromotedWidgets(node), ...getActivePreviewWidgets(node)] }) const activePromotedWidgets = computed({ diff --git a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts index aba1353a1d..954d53cd28 100644 --- a/src/core/graph/subgraph/migration/proxyWidgetMigration.ts +++ b/src/core/graph/subgraph/migration/proxyWidgetMigration.ts @@ -24,6 +24,7 @@ import type { IBaseWidget, TWidgetValue } from '@/lib/litegraph/src/types/widgets' +import { isWidgetValue } from '@/lib/litegraph/src/types/widgets' import { usePreviewExposureStore } from '@/stores/previewExposureStore' /** @@ -206,7 +207,12 @@ function pickHostValue( ) { return { value: undefined, isHole: true } } - return { value: hostWidgetValues[index] as TWidgetValue, isHole: false } + // Narrow rather than cast: a slot whose payload isn't a `TWidgetValue` + // (e.g. a function or `null` from corrupted serialization) is treated as + // a hole so the migration falls back to seeding from the source widget. + const raw = hostWidgetValues[index] + if (!isWidgetValue(raw)) return { value: undefined, isHole: true } + return { value: raw, isHole: false } } function findHostInputForLinkedSource( diff --git a/src/core/graph/subgraph/promotedWidgetView.ts b/src/core/graph/subgraph/promotedWidgetView.ts index 3c8c20929c..d3f493074c 100644 --- a/src/core/graph/subgraph/promotedWidgetView.ts +++ b/src/core/graph/subgraph/promotedWidgetView.ts @@ -4,6 +4,7 @@ import type { CanvasPointer } from '@/lib/litegraph/src/CanvasPointer' import type { Point } from '@/lib/litegraph/src/interfaces' import type { CanvasPointerEvent } from '@/lib/litegraph/src/types/events' import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' +import { isWidgetValue } from '@/lib/litegraph/src/types/widgets' import type { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' import type { BaseWidget } from '@/lib/litegraph/src/widgets/BaseWidget' import { toConcreteWidget } from '@/lib/litegraph/src/widgets/widgetMap' @@ -37,14 +38,6 @@ interface SubgraphSlotRef { displayName?: string } -function isWidgetValue(value: unknown): value is IBaseWidget['value'] { - if (value === undefined) return true - if (typeof value === 'string') return true - if (typeof value === 'number') return true - if (typeof value === 'boolean') return true - return value !== null && typeof value === 'object' -} - function isValueControlWidget(widget: IBaseWidget): boolean { return ( (widget as Record)[IS_CONTROL_WIDGET] === true && diff --git a/src/core/graph/subgraph/promotionUtils.ts b/src/core/graph/subgraph/promotionUtils.ts index bcdd41fb4c..567ad75288 100644 --- a/src/core/graph/subgraph/promotionUtils.ts +++ b/src/core/graph/subgraph/promotionUtils.ts @@ -2,12 +2,11 @@ import * as Sentry from '@sentry/vue' import type { PromotedWidgetSource } from '@/core/graph/subgraph/promotedWidgetTypes' import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes' import { t } from '@/i18n' -import type { - IContextMenuValue, - LGraphNode -} from '@/lib/litegraph/src/litegraph' +import type { IContextMenuValue } from '@/lib/litegraph/src/litegraph' +import { LGraphNode } from '@/lib/litegraph/src/litegraph' import type { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' -import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets.ts' +import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' +import { isWidgetValue } from '@/lib/litegraph/src/types/widgets' import { nextUniqueName } from '@/lib/litegraph/src/strings' import { useToastStore } from '@/platform/updates/common/toastStore' import { @@ -25,14 +24,6 @@ type PartialNode = Pick export type WidgetItem = [LGraphNode, IBaseWidget] export { CANVAS_IMAGE_PREVIEW_WIDGET } -function isWidgetValue(value: unknown): value is IBaseWidget['value'] { - if (value === undefined) return true - if (typeof value === 'string') return true - if (typeof value === 'number') return true - if (typeof value === 'boolean') return true - return value !== null && typeof value === 'object' -} - export function getWidgetName(w: IBaseWidget): string { return isPromotedWidgetView(w) ? w.sourceWidgetName : w.name } @@ -335,28 +326,23 @@ export function promoteWidget( parents: SubgraphNode[] ) { const source = toPromotionSource(node, widget) + // Both downstream helpers (`promotePreviewViaExposure`, + // `promoteValueWidgetViaSubgraphInput`) require the full `LGraphNode` + // shape — a `Pick<...>` won't do. Narrow once with `instanceof` rather + // than re-checking each call site with property guards + casts. + if (!(node instanceof LGraphNode)) return for (const parent of parents) { if (isPreviewPseudoWidget(widget)) { - promotePreviewViaExposure( - parent, - node as LGraphNode, - source.sourceWidgetName - ) + promotePreviewViaExposure(parent, node, source.sourceWidgetName) continue } - if ('getSlotFromWidget' in node) { - const result = promoteValueWidgetViaSubgraphInput( - parent, - node as LGraphNode, - widget - ) - if (!result.ok) { - Sentry.addBreadcrumb({ - category: 'subgraph', - level: 'warning', - message: `Failed to promote widget "${source.sourceWidgetName}" on node ${node.id}: ${result.reason}` - }) - } + const result = promoteValueWidgetViaSubgraphInput(parent, node, widget) + if (!result.ok) { + Sentry.addBreadcrumb({ + category: 'subgraph', + level: 'warning', + message: `Failed to promote widget "${source.sourceWidgetName}" on node ${node.id}: ${result.reason}` + }) } } refreshPromotedWidgetRendering(parents) diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index ec4b9c9b6f..d0733fe98d 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -76,6 +76,7 @@ import type { LGraphTriggerHandler, LGraphTriggerParam } from './types/graphTriggers' +import { LGraphTriggerActions } from './types/graphTriggers' import type { ExportedSubgraph, ExposedWidget, @@ -96,6 +97,9 @@ export type { LGraphTriggerParam } from './types/graphTriggers' +/** Runtime allowlist for {@link LGraph.trigger}, derived from {@link LGraphTriggerActions}. */ +const validTriggerActions = new Set(LGraphTriggerActions) + export type RendererType = 'LG' | 'Vue' | 'Vue-corrected' /** @@ -1362,13 +1366,7 @@ export class LGraph ): void trigger(action: string, param: unknown): void trigger(action: string, param: unknown) { - const validEventTypes = new Set([ - 'node:slot-links:changed', - 'node:slot-errors:changed', - 'node:property:changed', - 'node:slot-label:changed' - ]) - if (!validEventTypes.has(action as LGraphTriggerAction)) return + if (!validTriggerActions.has(action as LGraphTriggerAction)) return if (!param || typeof param !== 'object') return this.onTrigger?.({ type: action, ...param } as LGraphTriggerEvent) diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.ts index 06bac215f4..c4f2d9513a 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.ts @@ -28,10 +28,8 @@ import type { ISerialisedNode } from '@/lib/litegraph/src/types/serialisation' import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums' -import type { - IBaseWidget, - TWidgetValue -} from '@/lib/litegraph/src/types/widgets' +import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' +import { isWidgetValue } from '@/lib/litegraph/src/types/widgets' import { createPromotedWidgetView, isPromotedWidgetView @@ -66,14 +64,6 @@ type LinkedPromotionEntry = PromotedWidgetSource & { // the SVG's internal stylesheet on every ctx.drawImage() call per frame. const workflowBitmapCache = createBitmapCache(workflowSvg, 32) -function isWidgetValue(value: unknown): value is TWidgetValue { - if (value === undefined) return true - if (typeof value === 'string') return true - if (typeof value === 'number') return true - if (typeof value === 'boolean') return true - return value !== null && typeof value === 'object' -} - /** * An instance of a {@link Subgraph}, displayed as a node on the containing (parent) graph. */ @@ -1154,6 +1144,24 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph { serialized.properties = serializedProperties + // Per ADR 0009, host SubgraphNodes only carry promoted widgets — non- + // promoted host widgets would be silently dropped here. Surface the + // unexpected case in dev so a future custom subclass that adds bare + // widgets isn't ignored without a trace. + if ( + import.meta.env?.DEV && + this.widgets.some((w) => !isPromotedWidgetView(w)) + ) { + console.warn( + `SubgraphNode ${this.id}: serialize() drops non-promoted host widgets ` + + `(${this.widgets + .filter((w) => !isPromotedWidgetView(w)) + .map((w) => w.name) + .join(', ')}); ` + + 'expected only PromotedWidgetView instances per ADR 0009.' + ) + } + const widgetValues = this.inputs.flatMap((input) => { const widget = input._widget if (!widget || !isPromotedWidgetView(widget)) return [] diff --git a/src/lib/litegraph/src/types/graphTriggers.ts b/src/lib/litegraph/src/types/graphTriggers.ts index ae53053f5c..ebce4de974 100644 --- a/src/lib/litegraph/src/types/graphTriggers.ts +++ b/src/lib/litegraph/src/types/graphTriggers.ts @@ -35,7 +35,20 @@ export type LGraphTriggerEvent = | NodeSlotLinksChangedEvent | NodeSlotLabelChangedEvent -export type LGraphTriggerAction = LGraphTriggerEvent['type'] +/** + * Single source of truth for actions accepted by `LGraph.trigger()`. + * Both the runtime allowlist (`LGraphTriggerActions`) and the static type + * (`LGraphTriggerAction`) derive from this tuple — adding a new action is + * a one-place change. Keep in lockstep with {@link LGraphTriggerEvent}. + */ +export const LGraphTriggerActions = [ + 'node:property:changed', + 'node:slot-errors:changed', + 'node:slot-links:changed', + 'node:slot-label:changed' +] as const satisfies readonly LGraphTriggerEvent['type'][] + +export type LGraphTriggerAction = (typeof LGraphTriggerActions)[number] export type LGraphTriggerParam = Extract< LGraphTriggerEvent, diff --git a/src/lib/litegraph/src/types/widgets.ts b/src/lib/litegraph/src/types/widgets.ts index f733e7bcc4..b77001b0f6 100644 --- a/src/lib/litegraph/src/types/widgets.ts +++ b/src/lib/litegraph/src/types/widgets.ts @@ -375,6 +375,21 @@ export interface IRangeWidget extends IBaseWidget< export type TWidgetType = IWidget['type'] export type TWidgetValue = IWidget['value'] +/** + * Runtime type guard for {@link TWidgetValue}. Accepts any value shape that a + * widget can legally hold: primitives (`string`, `number`, `boolean`), + * non-null objects (arrays, plain objects), or `undefined`. Rejects `null` and + * functions. Used at serialization / migration boundaries that consume + * `unknown` payloads (e.g. `widgets_values`). + */ +export function isWidgetValue(value: unknown): value is TWidgetValue { + if (value === undefined) return true + if (typeof value === 'string') return true + if (typeof value === 'number') return true + if (typeof value === 'boolean') return true + return value !== null && typeof value === 'object' +} + /** * The base type for all widgets. Should not be implemented directly. * @template TValue The type of value this widget holds. diff --git a/src/stores/appModeStore.ts b/src/stores/appModeStore.ts index 7a16d50aac..d11c24dafe 100644 --- a/src/stores/appModeStore.ts +++ b/src/stores/appModeStore.ts @@ -267,9 +267,14 @@ export const useAppModeStore = defineStore('appMode', () => { function updateInputConfig(widget: IBaseWidget, config: InputWidgetConfig) { const targetEntityId = widget.entityId if (!targetEntityId) return - const entry = selectedInputs.value.find(([id]) => id === targetEntityId) - if (!entry) return - entry[2] = { ...entry[2], ...config } + const index = selectedInputs.value.findIndex( + ([id]) => id === targetEntityId + ) + if (index === -1) return + // Replace the tuple rather than mutating its `[2]` slot in place so + // reactive consumers that key off entry identity see the change. + const [id, type, options] = selectedInputs.value[index] + selectedInputs.value.splice(index, 1, [id, type, { ...options, ...config }]) } return {