fix: suppress link rendering during slot sync after graph reconfigure#8367
Merged
christian-byrne merged 7 commits intomainfrom Feb 3, 2026
Merged
fix: suppress link rendering during slot sync after graph reconfigure#8367christian-byrne merged 7 commits intomainfrom
christian-byrne merged 7 commits intomainfrom
Conversation
dosubot
bot
added
the
size:L
🎭 Playwright Tests: ✅ PassedResults: 507 passed, 0 failed, 0 flaky, 8 skipped (Total: 515) 📊 Browser Reports
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/30/2026, 02:02:29 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a pending-slot-sync flag to layout store, re-asserts the flag when scheduling slot syncs, exports and updates flushScheduledSlotLayoutSync to manage the flag and canvas dirtying, and prevents link rendering in LGraphCanvas while slot positions are pending. Changes
Sequence DiagramsequenceDiagram
participant App as App (configure)
participant Layout as layoutStore
participant SlotTrack as useSlotElementTracking
participant Canvas as LGraphCanvas
App->>Layout: setPendingSlotSync(true)
App->>SlotTrack: scheduleSlotLayoutSync()
SlotTrack->>Layout: setPendingSlotSync(true)
Note over Canvas: Canvas checks pending flag and\nskips link rendering while true (clears visible reroute set)
App->>SlotTrack: flushScheduledSlotLayoutSync()
SlotTrack->>SlotTrack: process pending slot nodes
SlotTrack->>Layout: setPendingSlotSync(false)
SlotTrack-->>App: return
App->>Canvas: markDirty()
Canvas->>Canvas: render links using synced slot positions
Suggested Reviewers
✨ 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 |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 26 kB (baseline 26 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 974 kB (baseline 974 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 471 kB (baseline 471 kB) • 🟢 -8 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.89 kB (baseline 2.89 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • ⚪ 0 BReusable component library chunks
Status: 4 added / 4 removed Data & Services — 2.71 MB (baseline 2.71 MB) • 🔴 +866 BStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 25.3 kB (baseline 25.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.1 MB (baseline 7.1 MB) • 🟢 -198 BBundles that do not match a named category
Status: 34 added / 34 removed |
![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: 2
🤖 Fix all issues with AI agents
In `@src/composables/graph/useGraphConfigureSlotSync.test.ts`:
- Around line 43-58: The test mutates LiteGraph.vueNodesMode directly and
restores it manually at the test end which can be skipped if the test throws;
change the test to restore vueNodesMode in an afterEach hook instead—capture the
original value of (LiteGraph as { vueNodesMode: boolean }).vueNodesMode before
each test (or in the test setup), and in afterEach reset it so
useGraphConfigureSlotSync tests never leave global state modified; reference the
LiteGraph.vueNodesMode mutation and the onGraphConfigured call in
useGraphConfigureSlotSync to locate where to capture and restore the original
value.
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts`:
- Around line 7-8: Remove the redundant "type-assertion" test that checks typeof
flushPendingSlotLayoutSyncs === 'function' — delete the it block named 'should
be a function' (which references flushPendingSlotLayoutSyncs) so tests only
cover behavior (e.g., the existing no-throw test). After removing that it block,
run the test suite and remove any now-unused imports related to the deleted test
if the linter flags them.
Comment on lines
43
to
58
| it('should not flush when vue nodes mode is disabled', async () => { | ||
| const { LiteGraph } = await import('@/lib/litegraph/src/litegraph') | ||
| ;(LiteGraph as { vueNodesMode: boolean }).vueNodesMode = false | ||
|
|
||
| const { flushPendingSlotLayoutSyncs } = | ||
| await import('@/renderer/extensions/vueNodes/composables/useSlotElementTracking') | ||
| const { useGraphConfigureSlotSync } = | ||
| await import('./useGraphConfigureSlotSync') | ||
|
|
||
| const { onGraphConfigured } = useGraphConfigureSlotSync() | ||
| onGraphConfigured() | ||
|
|
||
| expect(flushPendingSlotLayoutSyncs).not.toHaveBeenCalled() | ||
|
|
||
| ;(LiteGraph as { vueNodesMode: boolean }).vueNodesMode = true | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using afterEach for mock state restoration.
The manual restoration of vueNodesMode on line 57 could be missed if the test throws before reaching it. Using afterEach ensures cleanup runs regardless of test outcome.
♻️ Proposed refactor
describe('useGraphConfigureSlotSync', () => {
+ let originalVueNodesMode: boolean
+
beforeEach(() => {
vi.clearAllMocks()
})
+ afterEach(async () => {
+ const { LiteGraph } = await import('@/lib/litegraph/src/litegraph')
+ ;(LiteGraph as { vueNodesMode: boolean }).vueNodesMode = true
+ })
// ... first test unchanged ...
it('should not flush when vue nodes mode is disabled', async () => {
const { LiteGraph } = await import('@/lib/litegraph/src/litegraph')
;(LiteGraph as { vueNodesMode: boolean }).vueNodesMode = false
const { flushPendingSlotLayoutSyncs } =
await import('@/renderer/extensions/vueNodes/composables/useSlotElementTracking')
const { useGraphConfigureSlotSync } =
await import('./useGraphConfigureSlotSync')
const { onGraphConfigured } = useGraphConfigureSlotSync()
onGraphConfigured()
expect(flushPendingSlotLayoutSyncs).not.toHaveBeenCalled()
-
- ;(LiteGraph as { vueNodesMode: boolean }).vueNodesMode = true
})
})As per coding guidelines: "Leverage Vitest utilities for mocking where possible and keep module mocks contained without global mutable state."
🧰 Tools
🪛 ESLint
[error] 44-44: Unable to resolve path to module '@/lib/litegraph/src/litegraph'.
(import-x/no-unresolved)
[error] 48-48: Unable to resolve path to module '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'.
(import-x/no-unresolved)
[error] 50-50: Unable to resolve path to module './useGraphConfigureSlotSync'.
(import-x/no-unresolved)
🤖 Prompt for AI Agents
In `@src/composables/graph/useGraphConfigureSlotSync.test.ts` around lines 43 -
58, The test mutates LiteGraph.vueNodesMode directly and restores it manually at
the test end which can be skipped if the test throws; change the test to restore
vueNodesMode in an afterEach hook instead—capture the original value of
(LiteGraph as { vueNodesMode: boolean }).vueNodesMode before each test (or in
the test setup), and in afterEach reset it so useGraphConfigureSlotSync tests
never leave global state modified; reference the LiteGraph.vueNodesMode mutation
and the onGraphConfigured call in useGraphConfigureSlotSync to locate where to
capture and restore the original value.
Comment on lines
7
to
8
| it('should be a function', () => { | ||
| expect(typeof flushPendingSlotLayoutSyncs).toBe('function') |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing the type-assertion test.
The test typeof flushPendingSlotLayoutSyncs === 'function' is a change-detector test that verifies existence rather than behavior. TypeScript's type system already guarantees this at compile time. The second test (no-throw behavior) provides the meaningful coverage.
As per coding guidelines: "Do not write change detector tests that just assert defaults are certain values."
🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts`
around lines 7 - 8, Remove the redundant "type-assertion" test that checks
typeof flushPendingSlotLayoutSyncs === 'function' — delete the it block named
'should be a function' (which references flushPendingSlotLayoutSyncs) so tests
only cover behavior (e.g., the existing no-throw test). After removing that it
block, run the test suite and remove any now-unused imports related to the
deleted test if the linter flags them.
christian-byrne
force-pushed
the
vue-nodes-link-unalign
branch
from
e2bc749 to
da0e8f3
Compare
dosubot
bot
added
size:S
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts`:
- Around line 11-13: Add tests that cover the active path of
flushScheduledSlotLayoutSync by mocking its dependencies: stub
useSharedCanvasPositionConversion to return a predictable convert function, mock
useNodeSlotRegistryStore to provide a registry with one or more pending nodes
(their slot ids and nodeRects), and spy on layoutStore (or its updateSlotLayout
method) to assert it is called with converted positions; include assertions that
pending sync entries are cleared after flushScheduledSlotLayoutSync runs and
that no calls occur when registry is empty. Use the functions/objects
flushScheduledSlotLayoutSync, useSharedCanvasPositionConversion,
useNodeSlotRegistryStore, and layoutStore in your mocks/spies so the test
targets the real behavior rather than only the no-op path.
Comment on lines
11
to
13
| it('should not throw when called with no pending syncs', () => { | ||
| expect(() => flushScheduledSlotLayoutSync()).not.toThrow() | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding more meaningful behavioral tests.
The current test only verifies the function doesn't throw when there are no pending syncs. While this provides minimal coverage for the early-return path, consider testing actual behavior with pending nodes when the module's dependencies can be mocked.
Do you want me to generate a more comprehensive test suite that mocks the dependencies (useSharedCanvasPositionConversion, useNodeSlotRegistryStore, layoutStore) to verify actual sync behavior?
🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts`
around lines 11 - 13, Add tests that cover the active path of
flushScheduledSlotLayoutSync by mocking its dependencies: stub
useSharedCanvasPositionConversion to return a predictable convert function, mock
useNodeSlotRegistryStore to provide a registry with one or more pending nodes
(their slot ids and nodeRects), and spy on layoutStore (or its updateSlotLayout
method) to assert it is called with converted positions; include assertions that
pending sync entries are cleared after flushScheduledSlotLayoutSync runs and
that no calls occur when registry is empty. Use the functions/objects
flushScheduledSlotLayoutSync, useSharedCanvasPositionConversion,
useNodeSlotRegistryStore, and layoutStore in your mocks/spies so the test
targets the real behavior rather than only the no-op path.
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/litegraph/src/LGraphNode.ts (1)
3942-3972: Exclude unsynced slots from bounds to avoid stale layout calculations.
#measureSlotreturns early when positions are unsynced, but#measureSlotsstill pushes those slots intocreateBounds, which can reuse staleboundingRectvalues and skew widget layout/hit‑testing while Vue positions are pending. Consider only adding slots that were actually measured.🛠️ Suggested fix
- `#measureSlot`( + `#measureSlot`( slot: NodeInputSlot | NodeOutputSlot, slotIndex: number, isInput: boolean - ): void { + ): boolean { const pos = isInput ? this.getInputPos(slotIndex) : this.getOutputPos(slotIndex) // Skip if position not yet synced (Vue mode) - if (!pos) return + if (!pos) return false slot.boundingRect[0] = pos[0] - LiteGraph.NODE_SLOT_HEIGHT * 0.5 slot.boundingRect[1] = pos[1] - LiteGraph.NODE_SLOT_HEIGHT * 0.5 slot.boundingRect[2] = slot.isWidgetInputSlot ? BaseWidget.margin : LiteGraph.NODE_SLOT_HEIGHT slot.boundingRect[3] = LiteGraph.NODE_SLOT_HEIGHT + return true } `#measureSlots`(): ReadOnlyRect | null { const slots: (NodeInputSlot | NodeOutputSlot)[] = [] for (const [slotIndex, slot] of this.#concreteInputs.entries()) { // Unrecognized nodes (Nodes with error) has inputs but no widgets. Treat // converted inputs as normal inputs. /** Widget input slots are handled in {`@link` layoutWidgetInputSlots} */ if (this.widgets?.length && isWidgetInputSlot(slot)) continue - this.#measureSlot(slot, slotIndex, true) - slots.push(slot) + if (this.#measureSlot(slot, slotIndex, true)) { + slots.push(slot) + } } for (const [slotIndex, slot] of this.#concreteOutputs.entries()) { - this.#measureSlot(slot, slotIndex, false) - slots.push(slot) + if (this.#measureSlot(slot, slotIndex, false)) { + slots.push(slot) + } }src/lib/litegraph/src/LGraphCanvas.ts (1)
5653-5671: Avoid theas Pointassertion in the fallback start position.The new undefined-position guards are good. However, Line 5664 uses a type assertion; prefer a typed tuple constant to keep assertions as a last resort.
♻️ Suggested refactor to avoid the type assertion
- const startPos = - outputId === -1 - ? ([start_node.pos[0] + 10, start_node.pos[1] + 10] as Point) - : LiteGraph.vueNodesMode - ? getSlotPosition(start_node, outputId, false) - : start_node.getOutputPos(outputId) + const fallbackStartPos: Point = [ + start_node.pos[0] + 10, + start_node.pos[1] + 10 + ] + const startPos = + outputId === -1 + ? fallbackStartPos + : LiteGraph.vueNodesMode + ? getSlotPosition(start_node, outputId, false) + : start_node.getOutputPos(outputId)Based on learnings: Type assertions are a last resort (acceptable for legacy code interop).
Also applies to: 5700-5706, 5730-5736, 5807-5814, 5833-5839
🤖 Fix all issues with AI agents
In `@plan.md`:
- Line 1: Add a top-level heading to the Markdown file to satisfy MD041 by
inserting a level-1 heading (for example "# Description") at the very top of
plan.md so the document starts with a top-level heading; ensure the heading text
matches the file's section purpose (e.g., "Description") and is the first
non-blank line.
plan.md
Outdated
| @@ -0,0 +1,21 @@ | |||
| ## Description | |||
Contributor
There was a problem hiding this comment.
Use a top‑level heading to satisfy MD041.
🛠️ Suggested fix
-## Description
+# Description📝 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
| ## Description | |
| # Description |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@plan.md` at line 1, Add a top-level heading to the Markdown file to satisfy
MD041 by inserting a level-1 heading (for example "# Description") at the very
top of plan.md so the document starts with a top-level heading; ensure the
heading text matches the file's section purpose (e.g., "Description") and is the
first non-blank line.
4d6453b to
5effa57
Compare
dosubot
bot
added
size:M
After graph reconfiguration (e.g., undo/redo), there's a timing gap where canvas renders links before Vue components have mounted and synced slot positions to layoutStore. This adds a pendingSlotSync flag to layoutStore that: - Is set to true at start of graph.onConfigure() - Is cleared after flushScheduledSlotLayoutSync() completes - Causes drawConnections() to skip link rendering when true This prevents links from rendering with stale/missing positions during the brief window between configure() completing and Vue components syncing. Amp-Thread-ID: https://ampcode.com/threads/T-019c084c-49dd-764b-8125-8938c42612c8 Co-authored-by: Amp <amp@ampcode.com>
christian-byrne
force-pushed
the
vue-nodes-link-unalign
branch
from
5effa57 to
299386a
Compare
christian-byrne
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/litegraph/src/LGraphCanvas.ts`:
- Around line 5617-5619: Before the early return in LGraphCanvas where you check
"if (LiteGraph.vueNodesMode && layoutStore.pendingSlotSync) return", clear the
visible reroute cache to avoid stale entries; specifically reset
this.#visibleReroutes (the private cache used for hit-testing) to an empty
collection before returning so reroutes are refreshed once slot positions sync.
benceruleanlu
previously approved these changes
Jan 29, 2026
Member
benceruleanlu
left a comment
benceruleanlu
left a comment
There was a problem hiding this comment.
LGTM after taking a look at issues mentioned
Also, did the agent forget to commit or push the unit tests mentioned? having those as a regression test would be nice
| if (pendingNodes.size === 0) return | ||
| export function flushScheduledSlotLayoutSync() { | ||
| if (pendingNodes.size === 0) { | ||
| // Even if no pending nodes, clear the flag (e.g., graph with no nodes) |
Member
There was a problem hiding this comment.
Possible race condition: flushScheduledSlotLayoutSync() is called synchronously right after onConfigure. If Vue slot elements haven’t mounted yet, pendingNodes.size === 0 here, so this block clears pendingSlotSync and returns. That re‑enables link rendering before any DOM-based slot positions exist, which can reintroduce the misaligned-links bug under load. pendingNodes only becomes non‑empty when useSlotElementTracking runs in onMounted and calls scheduleSlotLayoutSync(). Consider only clearing the flag after a real flush (non-empty), or setting pendingSlotSync to true in scheduleSlotLayoutSync() so late mounts re‑assert it.
Contributor
Author
There was a problem hiding this comment.
Addressed in commit 2e1845b - scheduleSlotLayoutSync() now re-asserts the pendingSlotSync flag for late-mounting Vue components, and we only clear the flag when there are actual nodes to process.
| private addAfterConfigureHandler(graph: LGraph) { | ||
| const { onConfigure } = graph | ||
| graph.onConfigure = function (...args) { | ||
| // Set pending sync flag to suppress link rendering until slots are synced |
Member
There was a problem hiding this comment.
Potential stuck state: if onConfigure (or callbacks it triggers) throws, pendingSlotSync is never cleared because the flush/clear logic is after the call. That can permanently suppress link rendering. Suggest wrapping the handler body in try/finally and clearing the flag in finally (or at least in a catch) so links don’t stay hidden on error paths.
Contributor
Author
There was a problem hiding this comment.
Addressed in commit c2d548b - wrapped the onConfigure handler body in try/finally to ensure the flush runs even if an error is thrown.
Clears #visibleReroutes before the early return to prevent stale reroute entries from affecting hit-testing while pendingSlotSync is true. Addresses CodeRabbit review feedback. Amp-Thread-ID: https://ampcode.com/threads/T-019c0be1-0c2e-752d-b793-c6687e36c61c Co-authored-by: Amp <amp@ampcode.com>
Re-asserts pendingSlotSync flag in scheduleSlotLayoutSync() so that Vue components mounting after the synchronous flushScheduledSlotLayoutSync() call in onConfigure will keep link rendering suppressed until their slots are synced. Only clears the flag when there are actual nodes to process, allowing the RAF callback to properly sync late mounts. Addresses review feedback from benceruleanlu. Amp-Thread-ID: https://ampcode.com/threads/T-019c0be1-0c2e-752d-b793-c6687e36c61c Co-authored-by: Amp <amp@ampcode.com>
Ensures pendingSlotSync flag is cleared even if an error is thrown during graph configuration. This prevents links from being permanently hidden if onConfigure or its callbacks throw an exception. Addresses review feedback from benceruleanlu. Amp-Thread-ID: https://ampcode.com/threads/T-019c0be1-0c2e-752d-b793-c6687e36c61c Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts`:
- Around line 37-51: flushScheduledSlotLayoutSync currently returns early when
pendingNodes.size === 0 without clearing layoutStore's pending flag, causing
pendingSlotSync to remain true for workflows with no Vue nodes; modify
flushScheduledSlotLayoutSync so that when pendingNodes.size === 0 you call
layoutStore.setPendingSlotSync(false) before returning (or alternatively only
set the pending flag where scheduleSlotLayoutSync actually enqueues work),
ensuring the pendingSlotSync flag is cleared even when no
RAF/scheduleSlotLayoutSync was ever invoked.
… Vue nodes When no Vue nodes exist, pendingNodes stays empty but pendingSlotSync was set to true in app.ts. Without clearing the flag in the early return path, link rendering would be permanently suppressed for non-Vue-node workflows. Amp-Thread-ID: https://ampcode.com/threads/T-019c0c55-76a9-7128-88ff-f9c7b428a2a3 Co-authored-by: Amp <amp@ampcode.com>
After clearing the pendingSlotSync flag, the canvas needs to be marked dirty to trigger a redraw. Without this, links wouldn't appear until user interaction because the canvas didn't know it needed to re-render. Amp-Thread-ID: https://ampcode.com/threads/T-019c0c55-76a9-7128-88ff-f9c7b428a2a3 Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts`:
- Line 15: The import "import { app } from '@/scripts/app'" in
useSlotElementTracking.ts is causing ESLint import-x/no-unresolved; either
update the project alias resolver to map "@" to the src directory including
src/scripts/app.ts, or change the import to a resolvable path (e.g., relative
import to the actual file) so the symbol app in useSlotElementTracking.ts
resolves; make the change where the import appears (useSlotElementTracking.ts)
or in the build/ESLint alias config so linting passes.
| import { LiteGraph } from '@/lib/litegraph/src/litegraph' | ||
| import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier' | ||
| import { layoutStore } from '@/renderer/core/layout/store/layoutStore' | ||
| import { app } from '@/scripts/app' |
Contributor
There was a problem hiding this comment.
Resolve the @/scripts/app import or lint will fail.
ESLint flags import-x/no-unresolved for this path. Please confirm the alias
resolver includes src/scripts/app.ts, or adjust the import to a resolvable
path.
🧰 Tools
🪛 ESLint
[error] 15-15: Unable to resolve path to module '@/scripts/app'.
(import-x/no-unresolved)
🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts` at
line 15, The import "import { app } from '@/scripts/app'" in
useSlotElementTracking.ts is causing ESLint import-x/no-unresolved; either
update the project alias resolver to map "@" to the src directory including
src/scripts/app.ts, or change the import to a resolvable path (e.g., relative
import to the actual file) so the symbol app in useSlotElementTracking.ts
resolves; make the change where the import appears (useSlotElementTracking.ts)
or in the build/ESLint alias config so linting passes.
Contributor
Author
There was a problem hiding this comment.
This is a false positive - the @/scripts/app import resolves correctly and lint passes locally with no errors. The project's alias resolver includes this path.
Contributor
There was a problem hiding this comment.
@christian-byrne, thank you for the clarification! I understand the import resolves correctly in your environment and the alias configuration is properly set up. I'll disregard this ESLint report.
🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing styles
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-28T02:00:53.680Z
Learning: Applies to **/*.vue : Define slots via template usage, not `defineSlots`
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-28T02:00:53.680Z
Learning: Applies to **/*.{js,ts,tsx,vue} : Unused imports are disallowed and must be removed
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid `ts-expect-error` - fix the underlying type issue instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-28T02:00:53.680Z
Learning: Applies to **/*.{ts,tsx,vue} : Never use `as any` type assertions; fix the underlying type issue instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-28T02:00:53.680Z
Learning: Applies to eslint.config.ts : Configure ESLint with Vue + TS rules, no floating promises, and unused imports disallowed
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/AGENTS.md:0-0
Timestamp: 2026-01-28T02:01:01.525Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Always import from the barrel export for subgraph code to avoid circular dependencies (import from '@/lib/litegraph/src/litegraph' instead of individual modules like '@/lib/litegraph/src/LGraph')
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{js,ts,vue} : Implement proper error propagation
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-27T20:51:47.936Z
Learning: Applies to src/**/*.{js,ts,vue} : Validate trusted sources before processing
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:21.073Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, linting rules enforce keeping `import type` statements separate from regular `import` statements, even when importing from the same module. Do not suggest consolidating them into a single import statement.
When the synchronous flush is called with no pending nodes, we now check if the graph has any nodes. If it does, we keep the flag set to allow Vue components to mount and re-assert it. If the graph is empty (no nodes), we clear the flag since no Vue components will mount. This fixes the regression where links rendered with stale positions because the flag was cleared before Vue components had a chance to sync. Amp-Thread-ID: https://ampcode.com/threads/T-019c0c55-76a9-7128-88ff-f9c7b428a2a3 Co-authored-by: Amp <amp@ampcode.com>
Contributor
Author
Review Comments AddressedPushed 3 commits addressing the review feedback: 1. CodeRabbit - Clear reroute cache (LGraphCanvas.ts)Commit 2. @benceruleanlu - Race condition with late mounts (useSlotElementTracking.ts)Commit 3. @benceruleanlu - Error handling stuck state (app.ts)Commit |
This was referenced Feb 5, 2026
Closed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Description
Fixes link alignment issues after undo/redo operations in Vue Nodes 2.0. When multiple connections exist from the same output to different nodes, performing an undo would cause the connections to become misaligned with their inputs.
Root Cause
When undo triggers
loadGraphData, the graph is reconfigured and Vue node components are destroyed and recreated. The new slot elements mount and schedule RAF-batched position syncs viascheduleSlotLayoutSync. However, links are drawn before the RAF batch completes, causinggetSlotPosition()to return stale/missing positions.Solution
flushPendingSlotLayoutSyncs()function fromuseSlotElementTracking.tsuseGraphConfigureSlotSynccomposable that flushes pending syncs after graph configurationaddAfterConfigureHandlerinapp.ts, called afteronAfterGraphConfiguredTesting
flushPendingSlotLayoutSyncsuseGraphConfigureSlotSynccomposable┆Issue is synchronized with this Notion page by Unito