search

Inedo / pgscan

Dependency scanner for ProGet.
MIT License
11 stars 5 forks source link

Support npm package aliases #14

Closed crotondo-dap closed 1 year ago

crotondo-dap commented 2 years ago

Hi,

the method ReadDependencies() in the class NpmDependencyScanner.cs returns the name and the version of a dependency listed in the package-lock.json. This leads to an error if a dependency is using package aliases.

I wanted to upload consumer information for the used npm packges of a project. The project has a transitive dependency to bootstrap through the package „devextreme-themebuilder“. You can see the dependencies of this package in the following screenshot: image The version format of the 'bootstrap-v4' and 'bootstrap-v5' dependency is different from the other version formats. Further, there are no packages with the name 'bootstrap-v4' or 'bootstrap-v5'. The referenced package is called 'bootstrap'. This dependencies are listed in the package-lock.json in the following format: image The pgscan publish task ran into the following error: image As I was not aware of npm package aliases I got in contact with the DevExpress support and asked if their dependencies are correct this way. So they told me about aliases and that they are working with them because they need to download the package 'bootstrap' in two different versions.

Apparently the dependency name can be whatever I want it to be as long as the version is of the format 'npm:package-name@version'.

Have you already considered supporting npm package aliases?

crotondo-dap commented 1 year ago

Hi,

this issue is still bothering us with ProGet 2022 and pgscan 1.3.1.

Pgscan does no longer return an error. It says 'Dependencies published'. But if I look into the package 'bootstrap' in version 4.6.1 on the ProGet server, no usages are listed. If I look into the packages of the project itself, I can see the following entry: image Unfortunately both entries are wrong. The package should be 'bootstrap' and the version should be '4.6.1'.

Have you already considered supporting npm package aliases?

nancinedo commented 1 year ago

Hi @crotondo-dap , it looks like this fell through the cracks. Sorry we're not great w/ GitHub issues.

I'll make sure we review this. I never heard about "package aliases" either. What a strange thing....

So it sounds like we should just modify ReadDependencies(), like this?

  1. Look for version string that starts with npm:
  2. Look for last instance of '@', and split string into two parts (name + version)
  3. Use those parts (name + version) instead

Does that logic seem to make sense?

crotondo-dap commented 1 year ago

Hi @nancinedo,

would it be easier for you if I created a ticket at MyInedo even for pgscan-related issues? Or should I create an Issue at github and at MyInedo? Please let me know what works best for you.

In my opinion, package aliases are very poorly documented. But the format 'npm:package-name@version' seems to be the only package alias format available. Therefore, your proposed logic seems to make sense.