Comfy-Org / Comfy-Org/ComfyUI_frontend
Geometry listener ratchet: 4 of 54 registration sites released on only some exit paths
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
Four listener/observer registration sites in the geometry surface have a release that only covers some exit paths. Denominator: **54 registration sites examined** at `5002fae1b12d44831a21367afa7c0f798f7e7a2c` across `renderer/core/layout/**`, `vueNodes/layout/**`, `vueNodes/interactions/resize/`, `minimap/**`, `notifyLayoutChanges.ts` and `useSelectionToolboxPosition.ts` — 33 auto-disposed by Vue setup scope, 6 explicitly released, 7 unreleased-by-design module singletons, 4 below.
`notifyLayoutChanges` is correctly disposed on both exits (unmount and mid-life renderer/canvas change) via `GraphCanvas.vue:293`, for contrast.
### 1. `useNodeResize.ts:245-247` and `:89` — pre-existing
```ts
245: const stopMoveListen = useEventListener('pointermove', handlePointerMove)
246: const stopUpListen = useEventListener('pointerup', handlePointerUp)
247: const stopCancelListen = useEventListener('pointercancel', cleanup)
```
Registered from inside `startResize`, invoked from a template `@pointerdown` handler, so there is no active effect scope and VueUse's `tryOnScopeDispose` is a no-op. The only release is `cleanup()`, which begins `if (!isResizing.value) return` (`:219`).
If `startResize` runs a second time before the first sequence ends — second corner handle, or multi-touch, neither of which pointer capture prevents — a second triplet registers while `isResizing` is already true. The first `pointerup` stops triplet #1 and sets `isResizing = false`; triplet #2's handlers then early-return forever and its three window listeners stay, retaining `nodeElement` and `target` (detached DOM). The same guard strands `stopShiftSync()` (`:227`), leaving the window `keydown`/`keyup` from `useShiftKeySync.ts:86,89` installed.
**This block is byte-identical to the merge base `6532665db9`.** Pre-existing, not an ECS regression. The file appears in #14246's diff only for an unrelated `rootGraphId` change at `:80-83`.
### 2. `useMinimapGraph.ts:218-223` — 5 sites, pre-existing gap
Four `g.events.addEventListener` registrations plus the `onConnectionChange` monkey-patch. Cleanup (`disposeListeners` `:202-207` / `cleanupEventListeners` `:226-239`) is reached on unmount, canvas swap, and graph→graph — but **not** on graph→`null`, because `useMinimap.ts:208` guards `if (newGraph && newGraph !== oldGraph)`. A later `destroy()` calls `cleanupEventListeners()` with no argument, resolves `graph.value === null`, and bails at `:228`. The five registrations survive for the life of that `LGraph`.
`useMinimap.ts` is **not** touched by #14246, so the gap is pre-existing; the registrations themselves were restructured on this branch.
### 3. `useNodeDrag.ts:107`
The auto-pan `useRafFn` is constructed inside `new AutoPanController` from a pointermove handler, so again no active scope. Released via `autoPan?.stop()` (`:308`) on both normal drag exits. But if the `createSharedComposable` scope is torn down mid-drag — every Vue node unmounts, or the renderer is toggled off during a drag — `scope.stop()` stops the `whenever` watch at `:320` without ever running `resetDragState`, and the rAF loop keeps ticking against a dead `ds`.
### 4. `useSelectionToolboxPosition.ts:225`
One-shot `requestAnimationFrame` in `handleDragEnd`, never captured, no `cancelAnimationFrame` in the file. `onUnmounted` (`:248`) only calls `resetMoreOptionsState()`. A drag ending on the same frame the toolbox unmounts runs `updateSelectionBounds()` and writes module-level refs post-unmount. Not a growing leak, but an unguarded post-unmount callback.
### Authorship
`git blame` on sites 1 and 4 lands on `aff7f2a296` — `fix: prevent Record Audio waveform from overflowing node bounds (#8070)`, which touched **2,238 files / +612,812 lines**. That is a repo-wide bulk import, not authorship, so I am not naming its committer for those lines. Site 2 blames to the #14246 branch's `refactor!: migrate entity state to dedicated stores`. Site 3 blames to `jaeone94`, `fix: defer node auto-pan until drag starts (#12654)` — a real commit.
Assigning to the #14246 branch owner as the person with the surrounding context; happy for sites 1 and 4 to be reassigned or split off.
Related: #15620, #15618, #15594, #15577. Found reviewing #14246 (slice C3, layout/geometry).
Contributor guide
Assessment
This issue has not been assessed yet.