Vue.draggable: TypeScript: Added types in 2.24.0 causing issues

[rijkvanzanten]

Since release v2.24.0, vuedraggable includes typing information for the package. While this is generally super useful to have included out of the box, it's causing a bit of a problem for me currently:

I'm getting an error from TypeScript originating in vuedraggable when trying to compile my app after updating to v2.24.0:

ERROR in /Users/rijk/[redacted]/node_modules/vuedraggable/src/vuedraggable.d.ts(8,5):
8:5 Property 'element' in type 'Draggable' is not assignable to the same property in base type 'object & Record<never, any> & Vue'.
  Type 'string' is not assignable to type 'Element | null'.
     6 |     noTransitionOnDrag: boolean;
     7 |
  >  8 |     element: string;
       |     ^
     9 |
    10 |     tag: string;
    11 |

Next to this, TypeScript keeps complaining that the Draggable class can't be used in components as Draggable can't be assigned to VueConstructor<Vue>*:

No overload matches this call.
  Overload 3 of 3, '(options: ComponentOptionsWithProps<{ collection: { type: StringConstructor; required: true; }; }, Data, Data, {}, {}, { collection: string; } & {}>): VueProxy<...>', gave the following error.
    Type 'typeof Draggable' is not assignable to type 'VueConstructor<Vue> | FunctionalComponentOptions<any, PropsDefinition<any>> | ComponentOptions<never, any, any, any, any, Record<...>> | AsyncComponentPromise<...> | AsyncComponentFactory<...>'.
      Type 'typeof Draggable' is not assignable to type 'VueConstructor<Vue>'.

* This is when using @vue/composition-api

[rijkvanzanten]

Looking further into this, it looks like the second error might be caused by the first as well. Trying to force the type by using:

import Vue, { VueConstructor } from 'vue';
import VueDraggable from 'vuedraggable';

const draggable = VueDraggable as VueConstructor<Vue>;

returns

Conversion of type 'typeof Draggable' to type 'VueConstructor<Vue>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'Draggable' is not comparable to type 'CombinedVueInstance<Vue, object, object, object, Record<never, any>>'.
    Type 'Draggable' is not comparable to type 'Vue'.
      The types of '$mount(...).element' are incompatible between these types.
        Type 'string' is not comparable to type 'Element | null'.

[rijkvanzanten]

By commenting out the element statement in vuedraggable.d.ts, things work as expected:

declare module 'vuedraggable' {
  import Vue from 'vue';

  class Draggable extends Vue {
    static install(vue: typeof Vue): void;

    noTransitionOnDrag: boolean;

    // element: string;

    tag: string;

    options: object;

    componentData: object;

    clone: any;

    move: any;

    list: any[];

    value: any[];
  }

  export = Draggable;
}

@Keimeno I know it was a while ago, but do you remember what this element key was supposed to represent? https://github.com/SortableJS/Vue.Draggable/commit/1309c31eebc956a3be4431eb4604b37dd2ab32dd

@David-Desmaisons would you accept a PR to get this resolved in a patch release? Was there a specific reason to use the less-detailed declaration over the one that has been floating around in issues / PRs (https://github.com/SortableJS/Vue.Draggable/pull/794)?

[Keimeno]

Got my reference for element: string; from here: vuedraggable.js#L130.

And there it only shows, that it accepts Strings. You are trying to pass an Object of type Element | null, which is incompatible with a String.

However if it works by passing an Object of type Element or null as well, it should be possible changing the line vuedraggable.d.ts#L8 to element: string | Element | null;.

Also; element seems to be deprecated. But tag might have the same problem in that case.

[rijkvanzanten]

I'm not passing manually here..

I think the Vue type internally has a field called element as well, which we're overriding here. That in turn causes the usage of the component to fail, as TypeScript now considers Draggable incompatible with Vue, as Vue has element: Element | null; which isn't compatible with element: string;

[Keimeno]

That's a good point, might as well be the reason for the deprecation of element. What's your suggestion? Setting the element type to Element | null? However I'm not sure if removing it entirely adds any breaking changes.

[rijkvanzanten]

I think simply removing element: string; from the vuedraggable.d.ts in favor of relying on the extended version coming from Vue is the easiest fix. That being said, there was another type definition file floating around that seems to be more detailed in what it exports, so it might be an idea to rely on that instead?

https://github.com/SortableJS/Vue.Draggable/issues/379#issuecomment-480109531

[Keimeno]

Exporting the MoveEvent type makes sense, that should probably be added.

However, could you please provide the tsconfig.json, your vue declaration file, and your implementation of vuedraggable, as I was not able to recreate this bug.

Vue Component:

<template>
  <div>
    <draggable v-model="myArray">
      <div v-for="(element, index) in elements" :key="index">
        {{ element }}
      </div>
    </draggable>
  </div>
</template>

<script lang="ts">
import Vue from "vue";
import Draggable from "vuedraggable";

export default Vue.extend({
  components: {
    Draggable,
  },
  data: () => ({
    elements: ["hello", "world"],
  }),
});
</script>

Vue Declaration:

declare module '*.vue' {
  import Vue from 'vue'
  export default Vue
}

tsconfig:

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "strict": true,
    "jsx": "preserve",
    "importHelpers": true,
    "moduleResolution": "node",
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "baseUrl": ".",
    "types": ["webpack-env"],
    "paths": {
      "@/*": ["src/*"]
    },
    "lib": ["esnext", "dom", "dom.iterable", "scripthost"]
  },
  "include": [
    "src/**/*.ts",
    "src/**/*.tsx",
    "src/**/*.vue",
    "tests/**/*.ts",
    "tests/**/*.tsx"
  ],
  "exclude": ["node_modules"]
}

[David-Desmaisons]

@Keimeno @rijkvanzanten I am not a typescript expert, please provide a PR to get this changes merged. Thanks.

[Keimeno]

@rijkvanzanten I've just seen that you're using the @vue/composition-api package, which does not have a stable release. That might be a backwards compatibility bug atm. However, I'll take a look into it within the next few days, especially into adding an event type.

[someone1]

Wanted to mention the following error I'm getting during builds with vuedraggable if any work is going to be done to the Typescript definition file:

25:3 An export assignment cannot be used in a module with other exported elements.
    23 |   }
    24 | 
  > 25 |   export = Draggable;
       |   ^
    26 | }
    27 | 
Version: typescript 3.9.7

[rijkvanzanten]

@someone1 Could it be that you still have a manually added declaration for vuedraggable in a .d.ts file in your own codebase?

[rijkvanzanten]

@Keimeno

I've just seen that you're using the @vue/composition-api package, which does not have a stable release.

That shouldn't matter though, as this case causes the same issue. Also, where it fails is on the conflict with the Vue type, which still comes from vue itself

[someone1]

@someone1 Could it be that you still have a manually added declaration for vuedraggable in a .d.ts file in your own codebase?

Very quick/astute, and yes, accurate observation. Apologies - thought I scanned for this but looks like I didn't catch it.

[Keimeno]

@rijkvanzanten can you please verify that the improved declaration file works on version 2.23.0.

declare module 'vuedraggable' {
  import Vue, { VueConstructor } from 'vue';

  type CombinedVueInstance<
    Instance extends Vue,
    Data,
    Methods,
    Computed,
    Props
  > = Data & Methods & Computed & Props & Instance;

  type ExtendedVue<
    Instance extends Vue,
    Data,
    Methods,
    Computed,
    Props
  > = VueConstructor<
    CombinedVueInstance<Instance, Data, Methods, Computed, Props> & Vue
  >;

  export type DraggedContext<T> = {
    index: number;
    futureIndex: number;
    element: T;
  };

  export type DropContext<T> = {
    index: number;
    component: Vue;
    element: T;
  };

  export type Rectangle = {
    top: number;
    right: number;
    bottom: number;
    left: number;
    width: number;
    height: number;
  };

  export type MoveEvent<T> = {
    originalEvent: DragEvent;
    dragged: Element;
    draggedContext: DraggedContext<T>;
    draggedRect: Rectangle;
    related: Element;
    relatedContext: DropContext<T>;
    relatedRect: Rectangle;
    from: Element;
    to: Element;
    willInsertAfter: boolean;
    isTrusted: boolean;
  };

  const draggable: ExtendedVue<
    Vue,
    {},
    {},
    {},
    {
      options: any;
      list: any[];
      value: any[];
      noTransitionOnDrag?: boolean;
      clone: any;
      tag?: string | null;
      move: any;
      componentData: any;
    }
  >;

  export default draggable;
}

I'll create the pull request once you've verified it.

[rijkvanzanten]

@Keimeno This one doesn't cause any issues on my end 🙂 🎉

[rille111]

Cool but when will the npm package be updated? Getting lots of errors in all our applications now..

[Keimeno]

@David-Desmaisons

[David-Desmaisons]

@Keimeno PR merged and release updated: check version 2.24.1