Skip to content

Only adjust the drag hoverCount on an actual hover transition - #4977

Merged
raineorshine merged 1 commit into
mainfrom
claude/dragleave-unmount-cleanup
Aug 18, 2026
Merged

Only adjust the drag hoverCount on an actual hover transition#4977
raineorshine merged 1 commit into
mainfrom
claude/dragleave-unmount-cleanup

Conversation

@raineorshine

Copy link
Copy Markdown
Contributor

Second finding from the audit prompted by #4969 (a cleanup that latched state because React re-runs cleanups on every dependency change, not only on unmount). Independent of #4975.

Problem

useDragLeave shares a module-level hoverCount across every drop target and debounces a clear of state.hoveringPath when it reaches zero. Two defects:

1. Every effect run was treated as a hover transition. Any run that was not a false → true change of isDeepHovering fell into the else branch and decremented the shared count:

if (isDeepHovering && !prevIsDeepHoveringRef.current) {
  hoverCount += 1
} else {
  hoverCount = Math.max(hoverCount - 1, 0)
  if (hoverCount === 0) debouncedSetHoveringPath?.()
}

The effect runs on mount, so any thought mounting mid-drag decrements the count. With one target genuinely hovered, the count goes to zero and hoveringPath is cleared — the blue drop indicator disappears while the cursor is still over the target.

2. The cleanup claimed to be an unmount handler but wasn't. It was commented // Cleanup on unmount while the effect listed four dependencies, so React ran it before every re-run. It also never decremented, so a drop target unmounted mid-drag leaked its count and hoveringPath was then never cleared.

Both are reproduced by the new tests, which fail on main:

× keeps hoveringPath while a drop target is hovered and an unrelated thought mounts
× clears hoveringPath when a hovered drop target unmounts

Solution

  • Guard the count on isDeepHovering !== isCountedRef.current, so only a real enter or leave adjusts it. Mounts and canDropThought changes no longer touch the shared count.
  • Move the release into its own effect with empty deps, where it decrements this target's contribution. The empty deps are what make it an unmount handler; that is why it can't be folded back into the main effect.

Verification

  • New hook tests: 4 pass with the fix; the two above fail without it. The other two are guards that the count still reaches zero on leave (they pass either way, so they are guarding, not proving).
  • yarn lint:tsc, eslint on the changed files
  • Full unit suite: 1748 passed, 0 failed.
  • docs/drag-and-drop.md updated with the counting rule.

Not verified: I could not run the Puppeteer drag-and-drop suite locally, so the change is verified against the hook's semantics and unit tests rather than a real drag. Worth a manual drag before merging, particularly nested drop zones (Thought and useDragAndDropSubThought each register a useDragLeave).

🤖 Generated with Claude Code

useDragLeave shares a module-level hoverCount across every drop target, and
debounces a clear of state.hoveringPath when it reaches zero. The effect treated
every run as a hover transition: any run that was not a false→true change of
isDeepHovering fell into the `else` branch and decremented the shared count.
That branch is reached on mount, so any thought mounting mid-drag decremented
the count — dropping it to zero and blanking the drop indicator while a target
was still hovered.

Its cleanup was commented "Cleanup on unmount" but listed four dependencies, so
React ran it before every re-run rather than only on unmount. It also never
decremented, so a drop target unmounted mid-drag leaked its count and
hoveringPath was never cleared.

Guard the count on `isDeepHovering !== isCountedRef.current` so only a real
enter or leave adjusts it, and move the release into its own effect with empty
deps, where it decrements this target's contribution. The empty deps are what
make it an unmount handler.

Adds hook tests for the two reproduced failures (an unrelated thought mounting
mid-hover; a hovered target unmounting), a guard that the count still reaches
zero on leave, and documents the counting rule in docs/drag-and-drop.md.

Co-Authored-By: Claude Opus 5 (unknown context) <noreply@anthropic.com>
@raineorshine
raineorshine merged commit 41ad6ad into main Aug 18, 2026
11 checks passed
@raineorshine
raineorshine deleted the claude/dragleave-unmount-cleanup branch August 18, 2026 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant