Typescript support for CLI projects

[Fermain]

Hi, is there a plan to add type declaration files for this project?

This allows for full compatibility with typescript projects.

[oliverfindl]

Hello,

unfortunately I'm not familiar with typescript or type definitions (yet). I tried to generate them according docs, but result wasn't good. So I tried to write them manually in separate branch and you can check result here.

Please let me know, if it's correct.

Thanks.

[Fermain]

The file looks good from skim reading, I will test it out with a fresh CLI TypeScript build later on today and let you know.

[hxhieu]

Thanks for the plugin and the initial .d.ts file but it doesn't really work for me, so I have modified this a bit and the below seems work for me.

This is based on latest vue@3.0.5 and I have also removed the axios dependency import.

// ~/src/@types/vue-svg-inline-plugin.d.ts

declare module 'vue-svg-inline-plugin' {
  import { App } from 'vue';

  interface VueSvgInlinePluginOptions {
    directive?: {
      name?: string;
      spriteModifierName?: string;
    };
    attributes?: {
      merge?: string[];
      add?: { name: string; value: string | number }[];
      data?: string[];
      remove?: string[];
    };
    cache?: {
      version?: string;
      persistent?: boolean;
      removeRevisions?: boolean;
    };
    intersectionObserverOptions?: any;
    axios?: any;
    xhtml?: boolean;
  }

  type InstallFunction = (app: App, options?: VueSvgInlinePluginOptions) => any;
  type VueSvgInlinePlugin = (InstallFunction & { install?: InstallFunction }) | { install: InstallFunction };

  const _default: VueSvgInlinePlugin;

  export { VueSvgInlinePluginOptions };
  export default _default;
}

[oliverfindl]

Hello,

your version looks much better than mine blind attempt! I'm not familiar with typescript much, but let me ask, why you used union instead of discriminating union in VueSvgInlinePlugin type. I looked at source of Vue@3 and its using discriminating union there.

I fiddled with your types, and It looks fine so far. I added back types for Vue@2 and Axios, because this plugin should work in Vue@2 projects too and optionally use pre-configured Axios instance for fetch requests.

I will try to test it more in next few days (or maybe during weekend) and I will publish modified version of types into this branch.

Thanks.

[hxhieu]

@oliverfindl thanks for the update and to answer your question - I just copy the union from your initial .d.ts. I only wrap the whole thing around a module so TS can see it.

Also since Axios is optional (and it should be), I suggest we just leave axios?: any? so this is not coupled with Axios (optional) package. If people need typed they can just cast axios as AxiosInstance on their code where it depends on Axios.

[oliverfindl]

Hello,

in my types I have discriminating union in place. So probably typo at your end. I don't know if it have any effect, because my VS Code shows me tooltip with basic union instead of discriminating union, even I have it clearly defined as discriminating union.

As for Axios, I prefer to have exact types used/defined. Code doesn't do enough checking if it is actually Axios instance. Better safe than sorry. If you are worried about packages size, there are already in place both Vue@2 and Vue@3, so some small package won't make much difference. Not to mention, most of apps are already using Axios for fetch request (instead of native fetch), so there will be no overhead. But I'm thinking about scaffolding interfaces that would only contain required properties and methods. As for Vue@2 and Vue@3, only directive method (I should add check for it) and version property are required. And for Axios, its only get method. This is not future-proof solution, but should suffice for purpose of this plugin.

Thanks.

[markusand]

hxhieu declaration seems to work well. Could that be applied?

[oliverfindl]

Hello,

can you also test this declaration file? It is more specific about Vue and Axios.

Thanks.

[markusand]

I get the error message unable to resolve path to module @vue/runtime-core/dist/runtime-core. I guess because I've got Vue 3 installed. Indeed import { App } from 'vue' is fine . I don't know how to deal with legacy Vue compatibility.

[oliverfindl]

Hello,

well, this is known caveat - you need to have installed Vue@2, Vue@3 (or just @vue/runtime-core package) and Axios to be able to import all necessary types.

Plugin dependencies should look like this: https://github.com/oliverfindl/vue-svg-inline-plugin/blob/9278982e94d22e59c3af8e6209777e572191c179/package.json#L48-L55

You can try it yourself like this:

$ mkdir plugin-test && cd plugin-test

$ npm init -y && npm install "https://github.com/oliverfindl/vue-svg-inline-plugin.git#type-declaration" --save # this is somehow still missing declaration file

$ wget -O ./node_modules/vue-svg-inline-plugin/src/index.d.ts https://raw.githubusercontent.com/oliverfindl/vue-svg-inline-plugin/type-declaration/src/index.d.ts

$ touch index.ts # now you can install Vue (2 or 3) and fiddle with ./node_modules/vue-svg-inline-plugin/src/index.d.ts

This is kinda issue, because this plugin supports both Vue@2 and Vue@3. Therefore it needs types from both versions of Vue, while user is using only one. If I choose only 1 version, other half of user-base will come here to complain. Another issue is with Axios, which is only optional dependency.

I can use declaration file proposed in this issue, but it won't solve much - only plugin options (which is one-time copy-paste operation), while not enforcing Vue or Axios types.

So what do you propose? Should I use declaration file proposed above or should we iterate declaration file with types for Vue (2 and 3) and Axios?

Thanks.

[markusand]

Vue 3 has been on the game for a long time now and is the default version. Actually, 2.7 has been tagged as the EOL version for Vue 2. It's your project and you must decide your prefered roadmap, but IMHO it's a waste of time to struggle a lot more with it.

One suggestion could be to drop support for Vue 2 on a new version 3.0 release that targets only Vue 3, leaving the current one for legacy.

[oliverfindl]

Hello,

I don't have exact numbers, but from my personal experience, there is still much more Vue 2 legacy apps than actual Vue 3 apps.

New major version is only logical solution for me, but unfortunately I failed to start using this plugin in my projects (I'm using vue-svg-inline-loader) and I have literally zero motivation to clean up code-base to remove Vue 2 specific code, update all dependencies and test/update/recreate all examples.

I will probably just create some declaration file as proposed above where I declare Vue and Axios as any. Maybe I will create new major version in future, but most probably not.

Thanks.

[oliverfindl]

Hello,

I just published new version with type declaration file to npm. You can update package and test it.

Thanks.

[oliverfindl]

Closing this issue.