search

snyk / nodejs-lockfile-parser

Generate a Snyk dependency tree from package-lock.json or yarn.lock file
Other
57 stars 28 forks source link

fix: display unknown-version instead of truncating package name when … #141

Closed xzhou-snyk closed 2 years ago

xzhou-snyk commented 2 years ago

What this does

For the following npm lock file where packages exist in requires list but miss the lock entries, the package resolution displays weirdly:

"parentPackage": {
  "version": "0.0.1",
    "requires": {
      "childPackage1": "^1.0.0",
      "childPackage2": "^1.0.0"
    }
}

// no package resolution for childPackage1, childPackage2

It displays as childPackage@childPackage1 and childPackage@childPackage2. This is due to that we take away the version info (the "^1.0.0" part) from the package in requires list when constructing the dep map, which makes sense because in theory there should be corresponding lock entries in the lock file for these packages which would contain the actual resolved version. Then indexOf('@') for the childPackage1 resolves to -1.

This PR slightly improves the display by showing unknown-version, i.e. childPackage1@unknown-version and childPackage2@unknown-version.

This PR also uses the npm-package-arg to parse the package name instead of relying on indexOf('@').

Screenshots

Screenshot 2022-06-02 at 17 56 57

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

JamesPatrickGill commented 2 years ago

Hi, wondering about the scenario that would lead to this lockfile being used, is this even a valid lockfile? I ask as I don't think we should be glossing over a broken lockfile and making it "work". IMO we should error out with invalid lock.

xzhou-snyk commented 2 years ago

Hi, wondering about the scenario that would lead to this lockfile being used, is this even a valid lockfile? I ask as I don't think we should be glossing over a broken lockfile and making it "work". IMO we should error out with invalid lock.

I had the same question as well! But I'm not that familiar with the business logic so wasn't sure if it should be accepted ... I'll double check!

-> update: considering that this file is currently accepted, it would be better not to suddenly reject the file, because otherwise next recurring test would throw error for all the currently supported projects with similar lock file

snyksec commented 2 years ago

:tada: This PR is included in version 1.38.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: