-
Notifications
You must be signed in to change notification settings - Fork 502
Ensure widgets always get a single callback #7579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3e381cf
0d35d8e
b4e49a6
0d53831
087fdc0
28cc476
f903d92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| * Provides event-driven reactivity with performance optimizations | ||
| */ | ||
| import { reactiveComputed } from '@vueuse/core' | ||
| import { reactive, shallowReactive } from 'vue' | ||
| import { customRef, reactive, shallowReactive } from 'vue' | ||
|
|
||
| import { useChainCallback } from '@/composables/functional/useChainCallback' | ||
| import { isProxyWidget } from '@/core/graph/subgraph/proxyWidget' | ||
|
|
@@ -90,6 +90,23 @@ export interface GraphNodeManager { | |
| cleanup(): void | ||
| } | ||
|
|
||
| function widgetWithVueTrack( | ||
| widget: IBaseWidget | ||
| ): asserts widget is IBaseWidget & { vueTrack: () => void } { | ||
| if (widget.vueTrack) return | ||
|
|
||
| customRef((track, trigger) => { | ||
| widget.callback = useChainCallback(widget.callback, trigger) | ||
| widget.vueTrack = track | ||
| return { get() {}, set() {} } | ||
Myestery marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }) | ||
| } | ||
| export function useReactiveWidgetValue(widget: IBaseWidget) { | ||
| widgetWithVueTrack(widget) | ||
| widget.vueTrack() | ||
| return widget.value | ||
| } | ||
AustinMroz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| function getControlWidget(widget: IBaseWidget): SafeControlWidget | undefined { | ||
| const cagWidget = widget.linkedWidgets?.find( | ||
| (w) => w.name == 'control_after_generate' | ||
|
|
@@ -106,40 +123,63 @@ function getNodeType(node: LGraphNode, widget: IBaseWidget) { | |
| return subNode?.type | ||
| } | ||
|
|
||
| /** | ||
| * Validates that a value is a valid WidgetValue type | ||
| */ | ||
| const normalizeWidgetValue = (value: unknown): WidgetValue => { | ||
| if (value === null || value === undefined || value === void 0) { | ||
| return undefined | ||
| } | ||
| if ( | ||
| typeof value === 'string' || | ||
| typeof value === 'number' || | ||
| typeof value === 'boolean' | ||
| ) { | ||
| return value | ||
| } | ||
| if (typeof value === 'object') { | ||
| // Check if it's a File array | ||
| if ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this check needed if we still return the value regardless?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, no. There's been some long term discussion about wanting to phase away from simplifiedWidget and Since this function is just being moved without changes. I lean towards keeping this PR focused on the reactivity issue. |
||
| Array.isArray(value) && | ||
| value.length > 0 && | ||
| value.every((item): item is File => item instanceof File) | ||
| ) { | ||
| return value | ||
| } | ||
AustinMroz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Otherwise it's a generic object | ||
| return value | ||
| } | ||
| // If none of the above, return undefined | ||
| console.warn(`Invalid widget value type: ${typeof value}`, value) | ||
| return undefined | ||
| } | ||
AustinMroz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| export function safeWidgetMapper( | ||
| node: LGraphNode, | ||
| slotMetadata: Map<string, WidgetSlotMetadata> | ||
| ): (widget: IBaseWidget) => SafeWidgetData { | ||
| const nodeDefStore = useNodeDefStore() | ||
| return function (widget) { | ||
| try { | ||
| // TODO: Use widget.getReactiveData() once TypeScript types are updated | ||
| let value = widget.value | ||
|
|
||
| // For combo widgets, if value is undefined, use the first option as default | ||
| if ( | ||
| value === undefined && | ||
| widget.type === 'combo' && | ||
| widget.options?.values && | ||
| Array.isArray(widget.options.values) && | ||
| widget.options.values.length > 0 | ||
| ) { | ||
| value = widget.options.values[0] | ||
| } | ||
| const spec = nodeDefStore.getInputSpecForWidget(node, widget.name) | ||
| const slotInfo = slotMetadata.get(widget.name) | ||
| const borderStyle = widget.promoted | ||
| ? 'ring ring-component-node-widget-promoted' | ||
| : widget.advanced | ||
| ? 'ring ring-component-node-widget-advanced' | ||
| : undefined | ||
| const callback = (v: unknown) => { | ||
| const value = normalizeWidgetValue(v) | ||
| widget.value = value ?? undefined | ||
| widget.callback?.(value) | ||
| } | ||
|
|
||
| return { | ||
| name: widget.name, | ||
| type: widget.type, | ||
| value: value, | ||
| value: useReactiveWidgetValue(widget), | ||
| borderStyle, | ||
| callback: widget.callback, | ||
| callback, | ||
| controlWidget: getControlWidget(widget), | ||
| isDOMWidget: isDOMWidget(widget), | ||
| label: widget.label, | ||
|
|
@@ -286,128 +326,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { | |
| return nodeRefs.get(id) | ||
| } | ||
|
|
||
| /** | ||
| * Validates that a value is a valid WidgetValue type | ||
| */ | ||
| const validateWidgetValue = (value: unknown): WidgetValue => { | ||
| if (value === null || value === undefined || value === void 0) { | ||
| return undefined | ||
| } | ||
| if ( | ||
| typeof value === 'string' || | ||
| typeof value === 'number' || | ||
| typeof value === 'boolean' | ||
| ) { | ||
| return value | ||
| } | ||
| if (typeof value === 'object') { | ||
| // Check if it's a File array | ||
| if ( | ||
| Array.isArray(value) && | ||
| value.length > 0 && | ||
| value.every((item): item is File => item instanceof File) | ||
| ) { | ||
| return value | ||
| } | ||
| // Otherwise it's a generic object | ||
| return value | ||
| } | ||
| // If none of the above, return undefined | ||
| console.warn(`Invalid widget value type: ${typeof value}`, value) | ||
| return undefined | ||
| } | ||
|
|
||
| /** | ||
| * Updates Vue state when widget values change | ||
| */ | ||
| const updateVueWidgetState = ( | ||
| nodeId: string, | ||
| widgetName: string, | ||
| value: unknown | ||
| ): void => { | ||
| try { | ||
| const currentData = vueNodeData.get(nodeId) | ||
| if (!currentData?.widgets) return | ||
|
|
||
| const updatedWidgets = currentData.widgets.map((w) => | ||
| w.name === widgetName ? { ...w, value: validateWidgetValue(value) } : w | ||
| ) | ||
| // Create a completely new object to ensure Vue reactivity triggers | ||
| const updatedData = { | ||
| ...currentData, | ||
| widgets: updatedWidgets | ||
| } | ||
|
|
||
| vueNodeData.set(nodeId, updatedData) | ||
| } catch (error) { | ||
| // Ignore widget update errors to prevent cascade failures | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Creates a wrapped callback for a widget that maintains LiteGraph/Vue sync | ||
| */ | ||
| const createWrappedWidgetCallback = ( | ||
| widget: IBaseWidget, // LiteGraph widget with minimal typing | ||
| originalCallback: ((value: unknown) => void) | undefined, | ||
| nodeId: string | ||
| ) => { | ||
| let updateInProgress = false | ||
|
|
||
| return (value: unknown) => { | ||
| if (updateInProgress) return | ||
| updateInProgress = true | ||
|
|
||
| try { | ||
| // 1. Update the widget value in LiteGraph (critical for LiteGraph state) | ||
| // Validate that the value is of an acceptable type | ||
| if ( | ||
| value !== null && | ||
| value !== undefined && | ||
| typeof value !== 'string' && | ||
| typeof value !== 'number' && | ||
| typeof value !== 'boolean' && | ||
| typeof value !== 'object' | ||
| ) { | ||
| console.warn(`Invalid widget value type: ${typeof value}`) | ||
| updateInProgress = false | ||
| return | ||
| } | ||
|
|
||
| // Always update widget.value to ensure sync | ||
| widget.value = value ?? undefined | ||
|
|
||
| // 2. Call the original callback if it exists | ||
| if (originalCallback && widget.type !== 'asset') { | ||
| originalCallback.call(widget, value) | ||
| } | ||
|
|
||
| // 3. Update Vue state to maintain synchronization | ||
| updateVueWidgetState(nodeId, widget.name, value) | ||
| } finally { | ||
| updateInProgress = false | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sets up widget callbacks for a node | ||
| */ | ||
| const setupNodeWidgetCallbacks = (node: LGraphNode) => { | ||
| if (!node.widgets) return | ||
|
|
||
| const nodeId = String(node.id) | ||
|
|
||
| node.widgets.forEach((widget) => { | ||
| const originalCallback = widget.callback | ||
| widget.callback = createWrappedWidgetCallback( | ||
| widget, | ||
| originalCallback, | ||
| nodeId | ||
| ) | ||
| }) | ||
| } | ||
|
|
||
| const syncWithGraph = () => { | ||
| if (!graph?._nodes) return | ||
|
|
||
|
|
@@ -428,9 +346,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { | |
| // Store non-reactive reference | ||
| nodeRefs.set(id, node) | ||
|
|
||
| // Set up widget callbacks BEFORE extracting data (critical order) | ||
| setupNodeWidgetCallbacks(node) | ||
|
|
||
| // Extract and store safe data for Vue | ||
| vueNodeData.set(id, extractVueNodeData(node)) | ||
| }) | ||
|
|
@@ -449,9 +364,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { | |
| // Store non-reactive reference to original node | ||
| nodeRefs.set(id, node) | ||
|
|
||
| // Set up widget callbacks BEFORE extracting data (critical order) | ||
| setupNodeWidgetCallbacks(node) | ||
|
|
||
| // Extract initial data for Vue (may be incomplete during graph configure) | ||
| vueNodeData.set(id, extractVueNodeData(node)) | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.