Make node inputs reactive in vue #7546
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -218,6 +218,15 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { | |||||||||||||||||||||||||||||||||||||||||
| reactiveWidgets.splice(0, reactiveWidgets.length, ...v) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
| const reactiveInputs = shallowReactive<INodeInputSlot[]>(node.inputs ?? []) | ||||||||||||||||||||||||||||||||||||||||||
| Object.defineProperty(node, 'inputs', { | ||||||||||||||||||||||||||||||||||||||||||
| get() { | ||||||||||||||||||||||||||||||||||||||||||
| return reactiveInputs | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| set(v) { | ||||||||||||||||||||||||||||||||||||||||||
| reactiveInputs.splice(0, reactiveInputs.length, ...v) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+221
to
+229
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial 🧩 Analysis chain🏁 Script executed: cat -n src/composables/graph/useGraphNodeManager.ts | head -250 | tail -50Repository: Comfy-Org/ComfyUI_frontend Length of output: 2020 🏁 Script executed: cat -n src/composables/graph/useGraphNodeManager.ts | head -230 | tail -30Repository: Comfy-Org/ComfyUI_frontend Length of output: 1265 🏁 Script executed: rg -n 'node\.inputs' --type=ts --type=vue -A 2 -B 2 | head -100Repository: Comfy-Org/ComfyUI_frontend Length of output: 97 🏁 Script executed: rg -n 'Object\.defineProperty' --type=ts --type=vue -A 5 | head -150Repository: Comfy-Org/ComfyUI_frontend Length of output: 97 🏁 Script executed: rg -n 'this\.inputs' src/lib/litegraph/ --type=ts -A 2 -B 2 | head -80Repository: Comfy-Org/ComfyUI_frontend Length of output: 5581 🏁 Script executed: rg -n 'Object\.keys|Object\.entries|Object\.getOwnProperty' --type=ts src/composables/ src/components/ | grep -i nodeRepository: Comfy-Org/ComfyUI_frontend Length of output: 281 🏁 Script executed: rg -n 'for.*in.*node|for.*in.*this' --type=ts src/lib/litegraph/ | head -30Repository: Comfy-Org/ComfyUI_frontend Length of output: 3572 🏁 Script executed: rg -n 'Object\.defineProperty.*node.*inputs|Object\.defineProperty.*node.*widgets' --type=tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 256 🏁 Script executed: rg -n 'node\.inputs' src/composables/ --type=ts -B 2 -A 2 | head -50Repository: Comfy-Org/ComfyUI_frontend Length of output: 2597 🏁 Script executed: fd useGraphNodeManager -type fRepository: Comfy-Org/ComfyUI_frontend Length of output: 240 🏁 Script executed: node << 'EOF'
const obj = {};
Object.defineProperty(obj, 'test', {
get() { return 42; }
});
const descriptor = Object.getOwnPropertyDescriptor(obj, 'test');
console.log('Default descriptor:', descriptor);
console.log('Can iterate with for...of:', [...Object.keys(obj), 'N/A']);
// Test for...of on array with property
const arr = [1, 2, 3];
Object.defineProperty(arr, 'custom', { get() { return 99; } });
for (const item of arr) {
console.log('for...of item:', item);
}
console.log('custom property in for...of:', 'custom' in arr ? arr.custom : 'not iterated');
EOFRepository: Comfy-Org/ComfyUI_frontend Length of output: 298 🏁 Script executed: rg -n 'shallowReactive' src/composables/graph/ --type=ts -B 2 -A 8 | head -80Repository: Comfy-Org/ComfyUI_frontend Length of output: 2476 🏁 Script executed: find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "useGraphNodeManager\|extractVueNodeData" 2>/dev/nullRepository: Comfy-Org/ComfyUI_frontend Length of output: 682 🏁 Script executed: rg -n 'Object\.defineProperty' src/ --type=ts -B 3 -A 3 | head -60Repository: Comfy-Org/ComfyUI_frontend Length of output: 5533 🏁 Script executed: rg -n 'node\.inputs\s*=' --type=ts src/composables/ src/components/Repository: Comfy-Org/ComfyUI_frontend Length of output: 52 🏁 Script executed: rg -n 'node\.widgets\s*=' --type=ts src/composables/ src/components/Repository: Comfy-Org/ComfyUI_frontend Length of output: 52 🏁 Script executed: grep -r "configurable\|enumerable" src/composables/ src/components/ --include="*.ts" 2>/dev/null | head -20Repository: Comfy-Org/ComfyUI_frontend Length of output: 207 The reactive properties for
Adding descriptor flags is optional, but if you want to follow defensive coding practices, apply this diff: const reactiveInputs = shallowReactive<INodeInputSlot[]>(node.inputs ?? [])
Object.defineProperty(node, 'inputs', {
+ configurable: true,
+ enumerable: true,
get() {
return reactiveInputs
},
set(v) {
reactiveInputs.splice(0, reactiveInputs.length, ...v)
}
})And similarly for 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const safeWidgets = reactiveComputed<SafeWidgetData[]>(() => { | ||||||||||||||||||||||||||||||||||||||||||
| node.inputs?.forEach((input, index) => { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -252,7 +261,7 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager { | |||||||||||||||||||||||||||||||||||||||||
| badges, | ||||||||||||||||||||||||||||||||||||||||||
| hasErrors: !!node.has_errors, | ||||||||||||||||||||||||||||||||||||||||||
| widgets: safeWidgets, | ||||||||||||||||||||||||||||||||||||||||||
| inputs: reactiveInputs, | ||||||||||||||||||||||||||||||||||||||||||
| outputs: node.outputs ? [...node.outputs] : undefined, | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+264
to
265
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get context around lines 264-265 in the target file
echo "=== File context around lines 264-265 ==="
wc -l src/composables/graph/useGraphNodeManager.tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 160 🏁 Script executed: #!/bin/bash
# Read the relevant section of the file
sed -n '240,290p' src/composables/graph/useGraphNodeManager.tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 1502 🏁 Script executed: #!/bin/bash
# Check if outputs are mutated dynamically like inputs
echo "=== Checking for node.outputs mutations ==="
rg -n -C3 --type=ts --type=vue 'node\.outputs\s*=|\.outputs\s*\[.*\]\s*=' || echo "No direct mutations found"
echo -e "\n=== Checking for outputs array mutations (push/pop/splice/etc) ==="
rg -n -C3 --type=ts --type=vue '\.outputs\.(push|pop|splice|shift|unshift)' || echo "No array method mutations found"
echo -e "\n=== Checking for outputs property access/mutation ==="
rg -n --type=ts --type=vue 'outputs' src/composables/graph/useGraphNodeManager.ts | head -20Repository: Comfy-Org/ComfyUI_frontend Length of output: 395 🏁 Script executed: #!/bin/bash
# Find where reactiveInputs is created in the file
sed -n '200,270p' src/composables/graph/useGraphNodeManager.tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 2316 🏁 Script executed: #!/bin/bash
# Search for reactiveInputs definition in the entire file
rg -n 'reactiveInputs' src/composables/graph/useGraphNodeManager.ts -B5 -A5Repository: Comfy-Org/ComfyUI_frontend Length of output: 1034 🏁 Script executed: #!/bin/bash
# Check broader usage of outputs in the codebase to see if it's modified anywhere
rg -n 'outputs' src/composables/graph/ --type=ts -A2 -B2 | head -80Repository: Comfy-Org/ComfyUI_frontend Length of output: 1312 🏁 Script executed: #!/bin/bash
# Search across entire src directory for outputs mutations/modifications
echo "=== Searching for outputs property assignments ==="
rg '\bnode\.outputs\s*=' src/ --type=ts -A2 -B2 || echo "No node.outputs assignments"
echo -e "\n=== Searching for outputs in returned data being modified ==="
rg 'vueNode\.outputs|nodeData\.outputs' src/ --type=ts -A2 -B2 | head -40
echo -e "\n=== Check if outputs are used in reactiveComputed like inputs ==="
rg 'node\.outputs' src/composables/graph/useGraphNodeManager.ts -B2 -A2Repository: Comfy-Org/ComfyUI_frontend Length of output: 1577 🏁 Script executed: #!/bin/bash
# Check how VueNodeData outputs are used in components
echo "=== Search for outputs usage in Vue components ==="
rg 'outputs' src/components/ --type=ts -l | head -10Repository: Comfy-Org/ComfyUI_frontend Length of output: 313 🏁 Script executed: #!/bin/bash
# Get the exact pattern used for reactiveInputs to verify the suggested diff
sed -n '221,229p' src/composables/graph/useGraphNodeManager.tsRepository: Comfy-Org/ComfyUI_frontend Length of output: 338 🏁 Script executed: #!/bin/bash
# Check for any comments or TODOs about outputs handling
rg -n 'outputs|TODO|FIXME' src/composables/graph/useGraphNodeManager.ts | grep -i 'output' | head -10Repository: Comfy-Org/ComfyUI_frontend Length of output: 237 🏁 Script executed: #!/bin/bash
# Verify the VueNodeData type definition to see outputs type
rg -n 'interface VueNodeData|type VueNodeData' src/composables/graph/useGraphNodeManager.ts -A20Repository: Comfy-Org/ComfyUI_frontend Length of output: 584 Apply the same reactive pattern to outputs for consistency with inputs. Outputs are currently returned as a shallow copy ( Apply the same reactive pattern used for inputs: const reactiveInputs = shallowReactive<INodeInputSlot[]>(node.inputs ?? [])
Object.defineProperty(node, 'inputs', {
get() {
return reactiveInputs
},
set(v) {
reactiveInputs.splice(0, reactiveInputs.length, ...v)
}
})
+const reactiveOutputs = shallowReactive<INodeOutputSlot[]>(node.outputs ?? [])
+Object.defineProperty(node, 'outputs', {
+ get() {
+ return reactiveOutputs
+ },
+ set(v) {
+ reactiveOutputs.splice(0, reactiveOutputs.length, ...v)
+ }
+})Update line 265: - outputs: node.outputs ? [...node.outputs] : undefined,
+ outputs: reactiveOutputs,📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. @coderabbitai this would be good to do on its own as a dedicated follow-up PR to keep this one focused.
Contributor
There was a problem hiding this comment. @DrJKL, that makes sense! Keeping this PR focused on inputs is a good call. Would you like me to create a GitHub issue to track the outputs reactivity work as a follow-up? I can include the context from this discussion and the suggested implementation approach. 🧠 Learnings used
Contributor
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: Length of output: 57 @DrJKL, I've created issue #7547 for making node outputs reactive and assigned it to @AustinMroz. 🔗 #7547 The issue includes the technical context about applying the same 🧠 Learnings used |
||||||||||||||||||||||||||||||||||||||||||
| flags: node.flags ? { ...node.flags } : undefined, | ||||||||||||||||||||||||||||||||||||||||||
| color: node.color || undefined, | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nit, but I'd love a more descriptive parameter name.