search

RetireJS / retire.js

scanner detecting the use of JavaScript libraries with known vulnerabilities. Can also generate an SBOM of the libraries it finds.
https://retirejs.github.io/retire.js/
Other
3.66k stars 414 forks source link

Complete type definitions for npm package #416

Closed mgillam closed 1 year ago

mgillam commented 1 year ago

Is your feature request related to a problem? Please describe. Use of retire.js as a node package (e.g. for use by a node app) isn't really presented as a supported use case (hence why I felt this was more a feature request than a bug), but it looks like it generally should be possible. The issue I've run into is a Typescript app will complain about a lack of type definitions, even though the node module is currently written in TS. Analysis of what's in my app's node_modules shows most of the type definitions are there, it's notably retire.js that's missing its retire.d.ts counterpart, resulting in the error state.

image

Describe the solution you'd like I pulled down the retire.js repo and copied the retire.d.ts into the appropriate directory in my app's node_modules subdirectory, and it appeared to solve the behavior noted above. When I ran the build script from retire.js/node/package.json, the resulting output had retire.d.ts in the lib directory, but it was a symlink to src/retire.d.ts, my suspicion is that this is the reason it's missing when pushed to NPM. I'm not certain of the reason for this behavior, but I think a solution is to copy the src/retire.d.ts to lib/retire.d.ts as part of the build script, rather than symlinking. I believe that would make the type definitions complete when the package is installed via npm.

Describe alternatives you've considered I've though of a few alternatives, but they all have the shortcoming that updates to retire.js would not be fully available to my app unless I take some manual action.

  1. I could possibly copy the retire.d.ts into my app's src directory, so the type definitions are locally available in my project. This doesn't keep the type definitions in step with the actual installed package version though.
  2. I could have a script defined to install my depedencies, that includes shallow-copying retire.d.ts from GitHub directly to my node_modules.
  3. I could fork the package and maintain the solution in my own fork of it. This is likely the direction I would take if you can't or don't want to apply the solution proposed above.

Additional context I can probably work out a way to make the proposed solution work, and open a PR for it, but I wanted to check first if that would be appropriate here. I like the copyfiles package for copying during the build step, but even a cp command might do the trick here as long as the build environments for this package are always *nix/mac/powershell.

eoftedal commented 1 year ago

If you can create a PR for it, I would be happy to merge it. I don't remember why I had to make it a symlink.

retire.js is kept as javascript in stead of being ported to TypeScript, because it's also used by the chrome extension.

mgillam commented 1 year ago

Yep, sounds good. I'll take a bit of time to test it out and verify that it works as I expect, and then will open the PR if it all looks good to me. Thanks!

eoftedal commented 1 year ago

Maybe a simple copying in the build action in package.json is all that is needed.

mgillam commented 1 year ago

I suspect so, will try it shortly.