createDraggable and createDroppable are not reactive

[deje1011]

Hi,

First of all, thanks a lot for building this library!

While using it to implement drag and drop in my current project, I ran into an issue regarding reactivity. Consider the following example:

const MyComp = function (props) {
    const draggable = createDraggable(props.item.id, { item: props.item });
};

When props.item changes, the draggable id and data will be outdated, leading to the wrong item being modified on drop.

One way to fix this is to re-render the component when the item.id changes:

<Show when={props.item.id} keyed>
    <MyComp item={props.item} />
</Show>

This way, the id will be in sync but the data might still be outdated.

Another way to fix this would be to pass reactive getters to createDraggable / createDroppable instead of values:

const createReactiveDroppable = (getId: Accessor<Id>, getData: Accessor<Record<string, any>>): Droppable => {
    if (!getData) {
        getData = () => ({});
    }

    const [state, {addDroppable, removeDroppable}] = useDragDropContext()!;
    const [node, setNode] = createSignal<HTMLElement | null>(null);

    createEffect(() => {
        const resolvedNode = node();

        if (resolvedNode) {
            const id = getId();
            const data = getData();
            // addDroppable both reads and writes to the dndState,
            // which leads to an infinite loop if we don't prevent tracking here.
            untrack(() => {
                addDroppable({
                    id,
                    node: resolvedNode,
                    layout: elementLayout(resolvedNode),
                    data,
                });
            });
            onCleanup(() => removeDroppable(id));
        }
    });

    const isActiveDroppable = () => state.active.droppableId === getId();
    const transform = () => state.droppables[getId()]?.transform || noopTransform();

    const droppable = Object.defineProperties(
        (element: HTMLElement, accessor?: () => {skipTransform?: boolean}) => {
            const config = accessor ? accessor() : {};

            setNode(element);

            if (!config.skipTransform) {
                createEffect(() => {
                    const resolvedTransform = transform();
                    if (transformsAreEqual(resolvedTransform, noopTransform())) {
                        element.style.removeProperty('transform');
                    } else {
                        const style = transformStyle(transform());
                        element.style.setProperty('transform', style.transform ?? null);
                    }
                });
            }
        },
        {
            ref: {
                enumerable: true,
                value: setNode,
            },
            isActiveDroppable: {
                enumerable: true,
                get: isActiveDroppable,
            },
            transform: {
                enumerable: true,
                get: transform,
            },
        },
    ) as Droppable;

    return droppable;
};

export {createReactiveDroppable};

But of course this would be a breaking change of the API.

const MyComp = function (props) {
    const draggable = createReactiveDraggable(() => props.item.id, () => ({ item: props.item }));
};

What do you think? Or maybe there is another way to solve this?

[martinpengellyphillips]

Hmm, I'm curious why I'm not seeing this breaking today in my examples / usage.

Can you share a reproducible example to illustrate the issue more clearly?

[deje1011]

I think the examples just never use the props to build the id or data of the draggable/droppable.

I adjusted the "Basic Drag & Drop" example and pass a selected animal to the draggable:
In onDragEnd, draggable.id will always be "Dog", no matter which animal was selected. Using createReactiveDraggable instead of createDraggable fixes that.

Hope this helps, let me know if I should explain it in more detail :)

import {
    DragDropProvider,
    DragDropSensors,
    type DragEventHandler,
    createDraggable,
    createDroppable,
} from '@thisbeyond/solid-dnd';
import {createSignal, Show} from 'solid-js';

const [getAnimals] = createSignal(['Dog', 'Cat', 'Rat', 'Elephant']);
const [getSelectedAnimal, setSelectedAnimal] = createSignal(getAnimals()[0]);

const selectNextAnimal = () => {
    const animals = getAnimals();
    const index = animals.indexOf(getSelectedAnimal());
    setSelectedAnimal(animals[(index + 1) % animals.length]);
};

const Draggable = props => {
    const draggable = createDraggable(props.animal);
    // const draggable = createReactiveDraggable(() => props.animal);
    return (
        <div use:draggable class='draggable'>
            {props.animal}
        </div>
    );
};

const Droppable = props => {
    const droppable = createDroppable(1);
    return (
        <div use:droppable class='droppable' classList={{'!droppable-accept': droppable.isActiveDroppable}}>
            Droppable.
            {props.children}
        </div>
    );
};

export const DragAndDropExample = () => {
    const [where, setWhere] = createSignal('outside');
    const [droppedAnimal, setDroppedAnimal] = createSignal(null);

    const onDragEnd: DragEventHandler = ({droppable, draggable}) => {
        setDroppedAnimal(draggable.id);
        if (droppable) {
            setWhere('inside');
        } else {
            setWhere('outside');
        }
    };

    return (
        <DragDropProvider onDragEnd={onDragEnd}>
            <button onClick={selectNextAnimal}>Next</button>
            <DragDropSensors />
            <div class='min-h-15'>
                <Show when={where() === 'outside'}>
                    <Draggable animal={getSelectedAnimal()} />
                </Show>
            </div>
            <Droppable>
                <Show when={where() === 'inside'}>
                    <Draggable animal={getSelectedAnimal()} />
                </Show>
            </Droppable>

            <Show when={droppedAnimal()}>
                <div>Dropped: {droppedAnimal()}</div>
            </Show>
        </DragDropProvider>
    );
};

[wadamek65]

Can confirm, I ran into the same issue with a very similar simple example:

const draggable = createDraggable(id(), { itemName: item.name, itemType: item.itemType });

The itemName and itemType are stale and always have the first value. If they change in the component, the old value will still be passed because it's not reactive. I checked the examples and the only ones where the data passed was being modified in any way were the ones using createSortable so not sure if the issue is just not present there.

This is my whole component if you're curious:

type ItemProps = {
    id: string;
};

export const Item: Component<ItemProps> = (props) => {
    const { treeStore, toggleCollapsed } = useTreeContext();
    const item = useItemContext();

    const id = () => props.id;

    const draggable = createDraggable(id(), { itemName: item.name, itemType: item.itemType });
    const droppable = createDroppable(id());

    let nameInputElement!: HTMLDivElement;
    let containerElement!: HTMLDivElement;

    const childrenIds = createMemo(() => {
        return childrenToSortedByTypeName(treeStore, props.id);
    });

    createEffect(() => createShortcuts(containerElement, nameInputElement));

    return (
        <div data-tree-item-container="container">
            <div ref={draggable.ref}>
                <div
                    use:droppable
                    data-tree-item={props.id}
                    data-tree-item-level={item.level}
                    class={`${droppable.isActiveDroppable ? 'bg-secondary bg-opacity-20' : ''} relative my-1 flex w-fit items-center text-nowrap rounded-md border-[1px] border-dashed border-secondary pl-2 pr-4 focus:bg-base-200 focus:bg-opacity-30 focus:outline focus:outline-2 focus:outline-secondary`}
                    tabIndex={0}
                    role="treeitem"
                    // style={transformStyle(draggable.transform)}
                    ref={containerElement}
                >
                    <Show when={!isRootId(props.id)}>
                        <div {...draggable.dragActivators} class="absolute -left-6 hover:cursor-grab">
                            <DotsSixVertical />
                        </div>
                    </Show>
                    <Show when={isDirectory(item)}>
                        <button onClick={() => toggleCollapsed(props.id)}>
                            {!item.isCollapsed ? <CaretDown /> : <CaretRight />}
                        </button>
                    </Show>
                    <div class="mx-1">{item.itemType === 'directory' ? <Directory /> : <File />}</div>
                    <NameInput focusElement={containerElement} ref={nameInputElement} />
                </div>
            </div>
            <Show when={!item.isCollapsed}>
                <div class="border-l-2 pl-8">
                    <For each={childrenIds()}>
                        {(childId) => (
                            <ItemProvider id={childId}>
                                <Item id={childId} />
                            </ItemProvider>
                        )}
                    </For>
                </div>
            </Show>
        </div>
    );
};

[martinpengellyphillips]

🤔 I've been thinking about this a bit more.

The current behaviour is in line with how Solid is designed - calls in a non-tracking context are not reactive by default. The sortable example works because the list itself is reactive and the components that have changed are recreated in that tracking context (and so createDraggable is called again for the new nodes).

Each draggable id should be unique to the instance of the draggable itself (the call to createDraggable), not the data it represents. So it doesn't make sense for this to change reactively without recalling createDraggable.

However, note that the data argument itself is just a pass through (and isn't used internally by solid-dnd at all). If you want to make it maintain reactivity then you can pass data that is reactive itself and access it in tracking context (e.g. the JSX). For example:

const draggable = createDraggable(id(), { 
    get itemName(): { return item.name }, 
    get itemType(): { return item.itemType } 
});

// Later in the JSX
<div>
{draggable.data.itemName} // Now reactive!
</div>

If item is already a reactive object then you could pass it directly instead without wrapping: createDraggable(id(), item)

Hope that helps and going to close this now as wontfix

[wadamek65]

Thank you for the explanation @martinpengellyphillips ! I've applied your suggestion and it does indeed work! Thanks 😊