fix: guard drag movement without container - #1087
Conversation
|
@nrps9909 is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughChangesuseDrag 拖拽稳定性
Estimated code review effort: 2 (简单) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change prevents drag-move handling from dereferencing an unavailable container while preserving cleanup behavior; no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Summary
Fixes #1063.
Why
The document-level move listener can run after
containerRef.currentbecomes null. It currently dereferences the ref unconditionally and throws while readinggetBoundingClientRect(). The listener now snapshots the container and returns only for that move when it is unavailable. The independent end handler still removes all listeners, callsfinishChange, and resets drag state.Validation
9982e48withTypeError: Cannot read properties of null (reading getBoundingClientRect)npm test -- tests/useDrag.test.tsx --runInBand— 1/1 passednpm test -- --runInBand— 6 suites, 120/120 tests, 5/5 snapshots passednpm run tscnpm run lintnpx eslint src/hooks/useDrag.ts tests/useDrag.test.tsxnpx prettier --check src/hooks/useDrag.ts tests/useDrag.test.tsxnpm run compilegit diff --checkThe full suite still prints pre-existing React
act(...)warnings in unchanged Range/Slider focus tests; it exits successfully and the new regression is warning-free.I checked all 34 open PRs and their complete changed-file lists before submission; none touches either target file. The older merged #210 documents the same ref-null failure class in the pre-hook implementation and establishes a regression-test precedent, while no current duplicate patch exists.
AI assistance disclosure: Codex was used to reproduce the exact-base failure, trace the move/end listener ownership, draft the focused guard and regression, audit open PR overlap, and run the validation above. All behavior, test counts, commit state, and duplicate checks are directly verifiable.
Summary by CodeRabbit