microsoft / rushstack

Monorepo for tools developed by the Rush Stack community
https://rushstack.io/
Other
5.96k stars 602 forks source link

[rush] Detect unused "@types" dependencies using "scan" #2616

Open stekycz opened 3 years ago

stekycz commented 3 years ago

Since the rush scan command detects unused dependencies as well it would be nice it would detect unused @types dependencies as well. For example when the dependency is removed but the related @types dev dependency is left in package.json.

I believe another iteration(s) over declaredDependencies should be good enough.

  1. Find declared @types dependencies
  2. Validate, if every @types dependency have its own regular dependency defined

The affected code

However, some @types packages should be omitted from this check. E.g. @types/node.

octogonz commented 3 years ago

CC @chengcyber who worked on rush scan recently

octogonz commented 3 years ago

However, some @types packages should be omitted from this check. E.g. @types/node.

  • Do you know about some other @types packages to omit?

  • Would it make sense to add another option to configure what @types packages should be ignored?

This metadata is owned by the DefinitelyTyped project. It doesn't appear to be explicitly recorded in the @types NPM packages. (?)

However I noticed that a number of @types packages have the special phrase // Type definitions for non-npm package in their entry point .d.ts file. Some examples:

So perhaps this data is accurately tracked in the DefinitlyTyped repo, just in a form that is somewhat awkward to access.

👋 @sandersn do you know if the // Type definitions for non-npm package string is consistently applied?

If so, then one idea would be to clone the DefinitlyTyped repo and write a script that scans for this string, and builds an inventory of all such package names. That could be written to a JSON file, and then we could publish a helper library (definitely-typed-metadata?) that exports a simple API for querying this data set. Periodically we would run the script again and publish a new release with the latest data.

Possibly somebody did this already? We should investigate that first. 😉

sandersn commented 3 years ago

Yes, it's checked by tests before PRs get merged. We don't maintain a persistent index of the nonNpm property, because it's not important to the publisher right now. (Actually, there's no persistent index of anything; the publisher regenerates it on each publish.)

It's pretty easy to search for the string 'for non-npm package', but you could also use @definitelytyped/header-parser and import parseHeader or parseHeaderOrFail from it (the former returns an object or an exception, the latter throws the exception for you). The success result from parseHeader has a property nonNpm: boolean.

You can use other packages in @definitelytyped to parse a DT clone, which would make it easier to come up with a persistent index. A good place to start is @definitelytyped/definitions-parser: https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/definitions-parser