pzavolinsky / ts-unused-exports

ts-unused-exports finds unused exported symbols in your Typescript project
MIT License
749 stars 49 forks source link

Vue support #162

Open Timmmm opened 4 years ago

Timmmm commented 4 years ago

Unfortunately this doesn't seem to detect Typescript code in .vue files out of the box.

To reproduce, create a basic Vue project (Vue CLI can do this), and then export something from a .ts file and import it in a .vue file. ts-prune will report it as unused.

My tsconfig.json file does include .vue files:

  "include": [
    ...
    "src/**/*.vue",
  ],

However I guess it doesn't understand Vue single file components. They are converted to Typescript during webpack compilation by the vue-loader plugin.

Timmmm commented 4 years ago

Ok I have implemented this, essentially all you have to do is change app.ts:22 to this:

    const result = ts.parseJsonConfigFileContent(
      parseJsonResult.config,
      ts.sys,
      basePath,
      undefined,
      undefined,
      undefined,
      [
        {
          extension: 'vue',
          isMixedContent: true,
          scriptKind: ts.ScriptKind.Deferred,
        },
      ],
    );

I added a test, however your test system does not seem to work? It seems like it counts the errors and checks they are equal to 2, but even when there are 3 errors it still says PASS. Weird. Anyway, if you are happy with this approach I will put up a pull request. I don't think there is any issue with unconditionally adding .vue as an extra extension - if you don't use Vue it will just do nothing.

mrseanryan commented 4 years ago

@Timmmm looks like a good start!

I think supporting Vue could take some more work? To me it looks somewhat similar to supporting tsx files, and that was quite some more work than it initially looked. In particular, to include support for dynamic imports.

So I'd recommend more unit tests - for inspiration, search the code for tsx or have a look at:

https://github.com/pzavolinsky/ts-unused-exports/blob/master/features/base-url-undefined.feature

https://github.com/pzavolinsky/ts-unused-exports/blob/master/features/base-url.feature

https://github.com/pzavolinsky/ts-unused-exports/blob/master/features/include-dynamic-imports.tsx.feature

https://github.com/pzavolinsky/ts-unused-exports/blob/master/features/index.feature

I don't know how much of the above would also need to be done for Vue but it seems good to cover with tests, just in case.

Also, when we add a feature like this, we also update the CHANGELOG.

Regards Sean

Timmmm commented 4 years ago

I don't think the Vue files need any special handling like TSX does - the bit in the <script> tag is pure ordinary Typescript. I'll have a look at adding more unit tests if I get a chance though. Any idea why the existing unit tests don't seem to work by the way (see my previous comment)?

Also, one caveat I found when using this is that it is common in Vue projects to have a shims-vue.d.ts file which means that you get a load of warnings about the default export from Vue components being unused, but honestly I think that is Vue's fault for having poor Typescript support. I would be inclined not to work around it, and just include it as a known issue with Vue.

mrseanryan commented 4 years ago

hi @Timmmm

1 - I see - I'm not that familiar with Vue

2 - Not sure why the tests don't fail for you - can you open a PR and mention this issue number.

3 - Hmm people probably will complain about that :) Workaround is to use ignoreFiles - but I think actually we should ALWAYS ignore that filename, until we have a better solution.

  1. Also at least 1 integration would be nice - see the example folder = https://github.com/pzavolinsky/ts-unused-exports/tree/master/example

Tim do you think you will you have time to support this feature? (open a PR, address above items, test it a bit with a real Vue project)

Otherwise I will need to think about how I can schedule this work ...

I guess learning Vue could be fun :)

Timmmm commented 4 years ago

Sure I will open a pull request...

I guess learning Vue could be fun :)

I wouldn't recommend it. React is definitely superior and I regret using Vue! We just didn't have much experience with either when choosing and if you search for Vue vs React you just find waffle about Vue being easy. Nobody says "oh by the way Vue can't really type check component properties which will lead to endless runtime type errors, and also Vue's reactivity system makes everything reactive at the drop of a hat so you end up with everything connected to everything else which is pretty much the definition of spaghetti code." /rant

mrseanryan commented 4 years ago

Thank you @Timmmm ! - added some comments on the PR

FlorianWendelborn commented 3 years ago

I’d also appreciate vue support. Are there any plans to pick this back up?

Timmmm commented 3 years ago

No I don't work with Vue anymore (mercifully). Sorry!

mrseanryan commented 3 years ago

Possibly I will pick it up later this year, unless of course someone else wants to have a go...

FlorianWendelborn commented 3 years ago

@Timmmm I envy you haha

mrseanryan commented 3 years ago

This particular issue is biggish one, and I don't really want to invest time on it, until the release process is improved.

@pzavolinsky this could be done via #208 or #48 or perhaps some other way?