Skip to content

Fix reactivity washing in refreshNodeSlots #7802

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.

Sign up for GitHub

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

Merged
AustinMroz merged 4 commits into from
Jan 11, 2026
Merged

Fix reactivity washing in refreshNodeSlots #7802

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d1529b
Fix reactivity washing in refreshNodeSlots
AustinMroz Dec 30, 2025
2888c7c
Add test
AustinMroz Dec 30, 2025
f2980b1
Test nits
AustinMroz Dec 30, 2025
d361c94
Merge branch 'main' into austin/fix-reactivity-washing
DrJKL Jan 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions src/composables/graph/useGraphNodeManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,17 +236,10 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
})

// Update only widgets with new slot metadata, keeping other widget data intact
const updatedWidgets = currentData.widgets?.map((widget) => {
for (const widget of currentData.widgets ?? []) {
const slotInfo = slotMetadata.get(widget.name)
return slotInfo ? { ...widget, slotMetadata: slotInfo } : widget
})

vueNodeData.set(nodeId, {
...currentData,
widgets: updatedWidgets,
inputs: nodeRef.inputs ? [...nodeRef.inputs] : undefined,
outputs: nodeRef.outputs ? [...nodeRef.outputs] : undefined
})
if (slotInfo) widget.slotMetadata = slotInfo

@DrJKL DrJKL Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting for the future, but this is the kind of thing that could be done PRN in the context of a WidgetDataStore.

}
Comment on lines +248 to +251

@coderabbitai coderabbitai Bot Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

LGTM - Reactivity fix is correct!

The in-place mutation correctly preserves Vue reactivity by avoiding the spread operator's object reconstruction. This ensures widgets remain reactive after connections are made.

💡 Optional: Consider clearing stale slot metadata

The current code only updates widgets that have slot info. If a widget previously had a slot that was later removed (rare edge case), the old slotMetadata would persist. Consider:

 for (const widget of currentData.widgets ?? []) {
-  const slotInfo = slotMetadata.get(widget.name)
-  if (slotInfo) widget.slotMetadata = slotInfo
+  widget.slotMetadata = slotMetadata.get(widget.name)
 }

This assigns undefined to widgets without slots, ensuring stale metadata is cleared. However, this may not be necessary if structural changes are handled elsewhere via extractVueNodeData, and could have minor performance implications.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (const widget of currentData.widgets ?? []) {
const slotInfo = slotMetadata.get(widget.name)
return slotInfo ? { ...widget, slotMetadata: slotInfo } : widget
})
vueNodeData.set(nodeId, {
...currentData,
widgets: updatedWidgets,
inputs: nodeRef.inputs ? [...nodeRef.inputs] : undefined,
outputs: nodeRef.outputs ? [...nodeRef.outputs] : undefined
})
if (slotInfo) widget.slotMetadata = slotInfo
}
for (const widget of currentData.widgets ?? []) {
widget.slotMetadata = slotMetadata.get(widget.name)
}
🤖 Prompt for AI Agents 
In @src/composables/graph/useGraphNodeManager.ts around lines 248 - 251, The
loop that assigns slot metadata to widgets only sets widget.slotMetadata when
slotInfo exists, which can leave stale metadata on widgets whose slots were
removed; update the loop over currentData.widgets to assign widget.slotMetadata
= slotInfo ?? undefined (or explicitly set to undefined when no slotInfo) so
stale metadata is cleared, referencing the currentData.widgets iteration and
slotMetadata lookup and ensuring this change aligns with extractVueNodeData
expectations.

}

// Extract safe data from LiteGraph node for Vue consumption
Expand Down
48 changes: 48 additions & 0 deletions tests-ui/tests/composables/graph/useGraphNodeManager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { setActivePinia } from 'pinia'
import { createTestingPinia } from '@pinia/testing'
import { describe, expect, it, vi } from 'vitest'
import { nextTick, watch } from 'vue'

import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager'
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums'

setActivePinia(createTestingPinia())
Comment thread
AustinMroz marked this conversation as resolved.

describe('Node Reactivity', () => {
it('should trigger on callback', async () => {
const graph = new LGraph()
const node = new LGraphNode('test')
node.addInput('input', 'INT')
node.addWidget('number', 'testnum', 2, () => undefined, {})
graph.add(node)

const { vueNodeData } = useGraphNodeManager(graph)
const onReactivityUpdate = vi.fn()
watch(vueNodeData, onReactivityUpdate)
expect(onReactivityUpdate).toHaveBeenCalledTimes(0)

node.widgets![0].callback!(2)
await nextTick()
expect(onReactivityUpdate).toHaveBeenCalledTimes(1)
})
it('should remain reactive after a connection is made', async () => {
const graph = new LGraph()
const node = new LGraphNode('test')
node.addInput('input', 'INT')
node.addWidget('number', 'testnum', 2, () => undefined, {})
graph.add(node)

const { vueNodeData } = useGraphNodeManager(graph)
const onReactivityUpdate = vi.fn()
graph.trigger('node:slot-links:changed', {
nodeId: '1',
slotType: NodeSlotType.INPUT
})
Comment thread
AustinMroz marked this conversation as resolved.
watch(vueNodeData, onReactivityUpdate)

@DrJKL DrJKL Dec 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think if the common Arrange pieces were pulled out, it might read a bit cleaner here.

Also, want to add the same negative assertion from lines 22-23 above in this one for consistency?

@AustinMroz AustinMroz Dec 30, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some mixed feelings about the negative assertion even in the first test (I'm not sure it would ever catch anything since there's not an await nextTick() to let reactivity setttle) but symmetry is good and establishing that the update should not be caused by the graph.trigger call is important.


node.widgets![0].callback!(2)
await nextTick()
expect(onReactivityUpdate).toHaveBeenCalledTimes(1)
})
})