Fix component widget state on graph change #7648
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughExtended widget filtering logic in the active widget derivation process to recognize Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Test Results⏰ Completed at: 12/20/2025, 04:25:43 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status
⏰ Started at: 12/19/2025, 11:01:38 PM UTC 🚀 Building Storybook
⏱️ Please wait while the Storybook build is in progress... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/scripts/app.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
src/**/*.{vue,ts}: Avoid usingrefandwatchtogether - usecomputedinstead if possible
Do not addrefif a prop would suffice; do not addcomputedif the ref/prop directly would work; usewatchonly whencomputedwon't accomplish the goal
Leverage VueUse functions for performance-enhancing styles
Files:
src/scripts/app.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Extract complex type definitions that are inlined in multiple related places - name them for reuse
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/scripts/app.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: ESLint rules: Vue + TS rules, no floating promises, unused imports disallowed, i18n raw text restrictions in templates
Use es-toolkit for utility functions
Avoid mutable state - prefer immutability and assignment at point of declaration
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/scripts/app.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/scripts/app.ts
src/**/*.{js,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively - no new JavaScript files
Files:
src/scripts/app.ts
src/**/*.{ts,tsx,vue,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx,vue,js}: Indent with 2 spaces, use single quotes, no trailing semicolons, line width 80 - see.prettierrc
Sort and group imports by plugin, runpnpm formatbefore committing
Files:
src/scripts/app.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use function expressions - use function declarations instead when possible
Files:
src/scripts/app.ts
🧠 Learnings (5)
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/scripts/app.ts
🧬 Code graph analysis (1)
src/scripts/app.ts (1)
src/scripts/domWidget.ts (2)
DOMWidgetImpl(216-290)ComponentWidgetImpl(292-334)
🔇 Additional comments (1)
src/scripts/app.ts (1)
44-48: LGTM: Import updated correctly.The addition of
ComponentWidgetImplto the imports is necessary for the instanceof check in the active widget filter. The import follows the correct pattern with type imports properly separated.
Comment on lines
+819
to
+822
| .filter( | ||
| (w) => | ||
| w instanceof DOMWidgetImpl || w instanceof ComponentWidgetImpl | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the repository structure and files
find . -type f -name "*.ts" | grep -E "(app|widget)" | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1068
🏁 Script executed:
#!/bin/bash
# First, locate the actual src/scripts/app.ts file
find . -path "*/src/scripts/app.ts" -type f
# Look at lines 819-822
if [ -f "./src/scripts/app.ts" ]; then
echo "=== Lines 819-822 of src/scripts/app.ts ==="
sed -n '815,825p' "./src/scripts/app.ts"
fiRepository: Comfy-Org/ComfyUI_frontend
Length of output: 464
🏁 Script executed:
#!/bin/bash
# Search for all instanceof DOMWidgetImpl patterns
echo "=== All instanceof DOMWidgetImpl patterns ==="
rg -n 'instanceof\s+DOMWidgetImpl' --type=ts
echo ""
echo "=== All instanceof ComponentWidgetImpl patterns ==="
rg -n 'instanceof\s+ComponentWidgetImpl' --type=ts
echo ""
echo "=== Class definitions for widget classes ==="
rg -n 'class\s+(DOMWidgetImpl|ComponentWidgetImpl|BaseDOMWidget)' --type=tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 897
🏁 Script executed:
#!/bin/bash
# Examine the proxyWidget.ts file to see the context around those lines
echo "=== proxyWidget.ts lines 100-120 ==="
sed -n '100,120p' ./src/core/graph/subgraph/proxyWidget.ts
echo ""
echo "=== Check class definitions and inheritance ==="
sed -n '119,135p' ./src/scripts/domWidget.ts
sed -n '216,230p' ./src/scripts/domWidget.ts
sed -n '292,310p' ./src/scripts/domWidget.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2303
🏁 Script executed:
#!/bin/bash
# Check what isProxyWidget does and understand the context better
echo "=== Look for isProxyWidget definition ==="
rg -n 'isProxyWidget|ProxyWidget' ./src/core/graph/subgraph/proxyWidget.ts | head -20
echo ""
echo "=== Check the broader context of useDomWidgetStore usage ==="
rg -n 'useDomWidgetStore' --type=ts | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3165
🏁 Script executed:
#!/bin/bash
# Let's understand the relationship between activeWidgets and the deactivateWidget/setWidget calls
echo "=== Check domWidgetStore to understand deactivateWidget and setWidget ==="
rg -n 'deactivateWidget|setWidget' ./src/stores/domWidgetStore.ts
echo ""
echo "=== Check if ComponentWidgetImpl widgets can be proxy widgets ==="
rg -n 'newProxyWidget' --type=ts -A 5 | head -40Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1297
🏁 Script executed:
#!/bin/bash
# Check what newProxyWidget returns - is it always DOMWidgetImpl or could it be ComponentWidgetImpl?
echo "=== newProxyWidget function signature and return type ==="
sed -n '139,175p' ./src/core/graph/subgraph/proxyWidget.ts
echo ""
echo "=== Check BaseDOMWidget type definition ==="
rg -n 'type BaseDOMWidget|interface BaseDOMWidget' --type=ts -A 5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2041
🏁 Script executed:
#!/bin/bash
# Check newProxyFromOverlay to understand what type it returns
echo "=== newProxyFromOverlay function ==="
rg -n 'function newProxyFromOverlay|newProxyFromOverlay' --type=ts -A 30 | head -50
echo ""
echo "=== Check the actual context where deactivateWidget is called ==="
sed -n '95,125p' ./src/core/graph/subgraph/proxyWidget.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 5356
🏁 Script executed:
#!/bin/bash
# Let's verify the relationship between proxy widgets and the instanceof checks
echo "=== Check if proxy widgets can be instances of DOMWidgetImpl ==="
rg -n 'Proxy' ./src/core/graph/subgraph/proxyWidget.ts | head -10
echo ""
echo "=== Look at the disconnectedWidget to understand what kind of widget it is ==="
rg -n 'disconnectedWidget' --type=ts -B 2 -A 5 | head -40
echo ""
echo "=== Summary: Check all active widget registrations ==="
rg -n 'registerWidget|unregisterWidget' ./src/stores/domWidgetStore.ts -B 2 -A 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 4547
🏁 Script executed:
#!/bin/bash
# Let's verify if the issue is that ComponentWidgetImpl can also be a linkedWidget
echo "=== Check how linkedWidget is created and what types it can be ==="
sed -n '165,176p' ./src/core/graph/subgraph/proxyWidget.ts
echo ""
echo "=== Confirm the pattern: look at the entire context of the deactivate call ==="
sed -n '100,120p' ./src/core/graph/subgraph/proxyWidget.ts
echo ""
echo "=== Check if there are any other widget type checks elsewhere ==="
rg -n 'instanceof.*Widget' --type=ts | grep -v 'DOMWidgetImpl\|ComponentWidgetImpl' | head -10Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1861
Address similar widget type filtering in proxy widget management.
The fix correctly extends active widget filtering in app.ts to include ComponentWidgetImpl alongside DOMWidgetImpl. However, similar filtering patterns exist in src/core/graph/subgraph/proxyWidget.ts at lines 105 and 115 that only check for DOMWidgetImpl. Since both DOMWidgetImpl and ComponentWidgetImpl extend BaseDOMWidgetImpl and are registered with useDomWidgetStore, these proxy widget operations should also handle both types for consistency:
- Line 105:
if (w instanceof DOMWidgetImpl)→ should also checkComponentWidgetImpl - Line 115:
if (isActiveGraph && w instanceof DOMWidgetImpl)→ should also checkComponentWidgetImpl
Update these checks to use w instanceof DOMWidgetImpl || w instanceof ComponentWidgetImpl or extract a shared type guard.
🤖 Prompt for AI Agents
In src/core/graph/subgraph/proxyWidget.ts around lines 105 to 115, the code only
checks for DOMWidgetImpl when handling proxy widget logic but must also include
ComponentWidgetImpl (both extend BaseDOMWidgetImpl); update the conditions at
line 105 and line 115 to check for either DOMWidgetImpl or ComponentWidgetImpl
(e.g., w instanceof DOMWidgetImpl || w instanceof ComponentWidgetImpl) or
replace both with a shared type guard that returns true for instances of either
class, then use that guard in both places to ensure consistent handling.
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.21 MB (baseline 3.2 MB) • 🔴 +8.18 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 998 kB (baseline 996 kB) • 🔴 +1.33 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 297 kB (baseline 297 kB) • 🔴 +2 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 184 kB (baseline 185 kB) • 🟢 -1.51 kBReusable component library chunks
Status: 10 added / 10 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 1.86 kB (baseline 1.86 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.46 MB (baseline 8.46 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 1 added / 1 removed Other — 3.44 MB (baseline 3.44 MB) • ⚪ 0 BBundles that do not match a named category
Status: 22 added / 22 removed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Component widgets (e.g. Load3D) in the root graph stay inactive after leaving a subgraph.
Changes
Screenshots
┆Issue is synchronized with this Notion page by Unito