Conversation
…sensors/plugins/modifiers Previously the manager was constructed with an empty config, so defaultPreset.sensors (including KeyboardSensor) was always instantiated and bound to draggables before the reactive effect corrected it. Destroying a sensor doesn't unbind listeners already bound to draggables, so the default KeyboardSensor kept handling events even when a custom or no keyboard sensor was passed. Now the manager is created with the actual props, matching the React and Vue providers.
|
clauderic
left a comment
clauderic
left a comment
There was a problem hiding this comment.
Summary
Fixes #2112 by constructing the Svelte DragDropProvider's DragDropManager with the initial plugins / sensors / modifiers props instead of an empty object, closing the window where draggables could bind the default preset's sensors before the $effect corrected them.
We verified this against the source and the fix is correct:
DragDropManager's constructor already resolves customizables itself (packages/dom/src/core/manager/manager.ts:42-46callsresolveCustomizableoninput.plugins/sensors/modifiers), so passing the raw props through is safe andundefinedstill falls back todefaultPreset.- This brings the Svelte adapter in line with React, which has always constructed with
new DragDropManager(input)(packages/react/src/core/context/DragDropProvider.tsx:81) and therefore never had this bug. - The
$effecton lines 53-57 remains correct for subsequent prop updates.
Nice, minimal fix — thank you for tracking down the root cause and writing it up so precisely in the issue.
Feedback
Blocking issues
-
Missing changeset. This modifies
packages/svelte/src/core/context/DragDropProvider.svelte, so it needs a.changeset/*.md:--- '@dnd-kit/svelte': patch --- Fixed `DragDropProvider` briefly initializing default sensors before applying the `sensors` prop, which caused draggables to permanently bind stale sensors.
Suggestions
- The removed comment (
// Create manager once; plugins/sensors/modifiers are synced reactively via $effect below) was useful context for why the$effectbelow exists. Consider keeping a trimmed version, e.g.// Seed the manager with the initial props so children never bind default sensors; later updates are synced via the $effect below. - A regression test would be valuable if it's feasible in the Svelte test setup — something that renders
<DragDropProvider sensors={[PointerSensor]}>with a draggable and asserts that a keyboard activation does not start a drag. Not a blocker if there's no existing harness for it.
Changeset
- Status: missing. Should be
'@dnd-kit/svelte': patch.
Overall
The change itself is exactly right and we'd be happy to see it land — just needs the changeset. Thanks for both the detailed issue and the fix.
[claude-review]
2 participants
Previously the manager was constructed with an empty config, so defaultPreset.sensors (including KeyboardSensor) was always instantiated and bound to draggables before the reactive effect corrected it. Destroying a sensor doesn't unbind listeners already bound to draggables, so the default KeyboardSensor kept handling events even when a custom or no keyboard sensor was passed. Now the manager is created with the actual props, matching the React and Vue providers.