Error when slot template renders component with multiple root nodes

[jawa-the-hutt]

TLDR: reporting a bug and have a proposed fix, but not sure how much of an edge case this is or not.

When inserting a component into the slot, if the component has multiple root nodes in it such that it doesn't get detected by the check at line 26 in renderHelper.js, then the __draggable_context property that gets added to the DOM element is added to the wrong node. When this happens, you cannot drag/drop correctly and get a throw error in the onDragStart event: Cannot read properties of null (reading 'element')

codesandbox.io link

https://codesandbox.io/s/bold-pike-xrddv

Step by step scenario

Take something like this for App.vue

<table class="table table-striped">
  <thead class="thead-dark">
    <tr>
      <th scope="col">Id</th>
      <th scope="col">First Name</th>
      <th scope="col">Last Name</th>
      <th scope="col">Age</th>
    </tr>
  </thead>
  <draggable v-model="list" tag="tbody" item-key="name">
    <template #item="{ element }">
      <example :element="element" />
    </template>
  </draggable>
</table>

and something like this for the Example.vue component:

<template>
  <!-- random comment here -->
  <tr :data-draggable="true">
    <td>{{ element.id }}</td>
    <td>{{ element.firstName }}</td>
    <td>{{ element.lastName }}</td>
    <td>{{ element.age }}</td>
  </tr>
</template>

The comment line in Example.vue causes the vue compiler to detect multiple root nodes and thus, it surrounds all nodes with an empty text element. So, something like this:

As you can see, this is the childNodes of the tbody element. Here is the first text node expanded and you can see the __draggable_context property has been added to it.

So, when the onDragStart event happens and it calls getContext here, it fails to find the element correctly and throws the exception.

Actual Solution

Here is a proposed solution at codesandbox.io: https://codesandbox.io/s/pedantic-joliot-drjm3.

Please note, for this sandbox, I ended up setting up VueDraggable locally within the sandbox as a subfolder. I do have a PR ready that is the same fix that's contained in this sandbox. You can see the proposed fix in the componentStructure.js file at the very beginning. I'm also manually assigning the data-draggable attribute where I need it as that's another symptom of the overall issue and that is that this attribute is not being assigned correctly.

const getHtmlElementFromNode = (node) => {
  const nodeType = node.el.nodeType;
  const parentElementCount = node.el.parentElement.childElementCount;
  const parentChildNodesCount = node.el.parentElement.childNodes.length;

  // we might have a text node fragment that Vue3 uses when the parent element
  // has multiple root nodes.
  // so we test for it here and if so, we return the next sibling as we don't want the
  // __draggable_context added to what is essentially an empty, unreachable page element
  if (nodeType === 3 && parentElementCount !== parentChildNodesCount) {
    node = node.el.nextElementSibling;
  }

  if (node.el) return node.el;
  else return node; // then we probably are returning the nextSibling.
};

What I attempt to do is detect these empty text nodes that Vue 3 is inserting and based on that, I go find the next Sibling and return it instead of the empty text node.

All the above said, I'm not 100% sure this is the best way to go about addressing this scenario. For me specifically, this situation came up with a 3rd party table library where I wanted to be able to drag/sort rows and it wasn't a comment in the template section, but it's use of multiple components via renderless functions and jsx elements to render a tr that Vue thinks has multiple root nodes. My use of the comment in the template section was my attempt to recreate the scenario.

Again, not sure I've gone about solving this the "best" way or if there is some other way that would be a better long term fix for this issue.

[jawa-the-hutt]

And of course, as soon as I type all that up and submit the issue, I start testing out different scenarios and discover an issue with the proposed fix. It get's the tr node and moves it, but the underlying childNodes list get's all out of order and the reliance of the overall code base of going after the el to add the __draggable_context onto the element causes other bugs.

Ultimately, it feels like the reliance on the el to add the __draggable_context is going to be an issue in this scenario and may ultimately be why this package tries to keep this situation cropping up at all by trying to detect more than one element in the slot template and throwing the error.

I think the long-term fix may be figuring out how to utilize template Refs to track the elements we want to drag/drop and not go after the underlying el, but that's just a guess. Not sure how that strategy will work or not as it relates to the underlying SortableJS library being used.

[jawa-the-hutt]

FWIW, I've created a 2nd example that more accurately reflects what I'm experiencing locally with the 3rd party table: https://codesandbox.io/s/quizzical-cookies-rih8u

For these multi-root components, Vue is managing the childNodes list somehow and with the drag/drop Vue is not getting updated on what's has changed so it's still getting some weird behavior.

[jmjurado23]

I have the same problem with slots and searching for a solution and I've found a PR about this. Is this PR a solution for our problem? #101

[jawa-the-hutt]

@jmjurado23 It wasn't a fix for the issue I had as I pulled the fork for that PR and tried it and it didn't work.

My understanding is that the usage of slots will automatically introduce the multiroot component. I've got a fork that fixes the issue with Multiroot components. But it also has several other improvements such as moves the repo to using Vite and away from webpack so that we get ESM modules which helps address #117 and a couple of other issues I've come across.

I tried to also convert things to using the composition API, but hit a wall on an issue. I've got the current options API version working. I reached out for some help on the Vue Discord but didn't get any clear direction on what the actual difference might be between the two API's that would cause things to work for the Options API but not the Composition API.

you can try it out here: https://github.com/jawa-the-hutt/vue.draggable.next

there's a bunch of cleanup that's needed on this, so it's not really production ready. I have not touched the Tests to see if they even work. Hence, why I haven't submitted this back up as a PR.

[yaolunmao]

Do you know why only el-button can be dragged

This is my project

https://codesandbox.io/s/github/yaolunmao/el-drag-test

I guess it may have something to do with inheritAttrs, but I can't solve it

[yaolunmao]

Do you know why only el-button can be dragged

This is my project

https://codesandbox.io/s/github/yaolunmao/el-drag-test

I guess it may have something to do with inheritAttrs, but I can't solve it

I tried to track the code and found that only el-button can have data-draggable and __draggable_context attributes, but I don't know why

[vxow]

Has this problem not been followed up?Can you expose a hook to customize the specified DOM