Skip to content

fix: throw 'not implemented' error in buildDepTree for npm v2/v3 lockfiles#200

Closed
milahu wants to merge 1 commit into
snyk:mainfrom
milahu:fix-deptree-parser-error-npm-v2-lockfile
Closed

fix: throw 'not implemented' error in buildDepTree for npm v2/v3 lockfiles#200
milahu wants to merge 1 commit into
snyk:mainfrom
milahu:fix-deptree-parser-error-npm-v2-lockfile

Conversation

@milahu
Copy link
Copy Markdown

@milahu milahu commented Aug 10, 2023

What this does

throw not implemented error in buildDepTree for npm v2/v3 lockfiles

better than OutOfSyncError from getDependencyTree

OutOfSyncError: Dependency @cycle/http was not found in package-lock.json. Your package.json and package-lock.json are probably out of sync. Please run "npm install" and try again.
    at PackageLockParser.getDependencyTree (node_modules/snyk-nodejs-lockfile-parser/dist/parsers/lock-parser-base.js:124:27)
    at async PackageLockParser.getDependencyTree (node_modules/snyk-nodejs-lockfile-parser/dist/parsers/package-lock-parser.js:28:32)
  code: 422,
  dependencyName: '@cycle/http',
  lockFileType: 'npm7'

problem in PackageLockParser.getDepMap:
packageLock.dependencies is undefined, it should use packageLock.packages

flattenLockfileRec(packageLock.dependencies || {}, []);

so the not implemented error is thrown in getDepMap before calling flattenLockfileRec

to implement support for v2/v3 npm lockfiles, a good place would be getDepMap

  protected getDepMapV2(packageLock: PackageLock): DepMap {
    // TODO implement
  }

  protected getDepMap(lockfile: Lockfile): DepMap {
    const packageLock = lockfile as PackageLock;

    if (packageLock.lockfileVersion == 2) {
      return this.getDepMapV2(packageLock);
    }

Notes for the reviewer

low priority

... for npm v2/v3 lockfiles

better than 'OutOfSyncError'
@milahu milahu requested a review from a team as a code owner August 10, 2023 14:10
@calderonth
Copy link
Copy Markdown

Does that mean that currently calling buildDepTree is expected to fail for npm v2/v3 lockfiles?

@milahu
Copy link
Copy Markdown
Author

milahu commented Aug 25, 2023

yes, see the readme

This is a small utility package that parses lock file and returns either a dependency tree or a dependency graph. Dependency graphs are the more modern data type and we plan to migrate fully over.

Dep graph generation supported for:

  • package-lock.json (at Versions 2 and 3)
  • yarn.lock

Legacy dep tree supported for:

  • package-lock.json
  • yarn 1 yarn.lock
  • yarn 2 yarn.lock

@calderonth
Copy link
Copy Markdown

ok so say for the time being I wanted to cover all manifest version I would have to add logic in order to detect manifest/lockfile types/versions and then call either dep-graph or dep-tree?

@milahu
Copy link
Copy Markdown
Author

milahu commented Aug 25, 2023

yes, thats what im doing in my pnpm-install-only

  const lockfileContent = read(lockfilePath);
  const lockfile = JSON.parse(lockfileContent);

  const [deps, walk_deps] = (
    lockfile.lockfileVersion == 3 ? await getDepgraph(lockfilePath) : // TODO verify
    lockfile.lockfileVersion == 2 ? await getDepgraph(lockfilePath) :
    lockfile.lockfileVersion == 1 ? await getDeptree(lockfilePath) :
    [null, null]
  )

  if (deps == null && walk_deps == null) {
    throw new Error('failed to recognize the lockfile type');
  }

edit: see also my parse-package-lock

@calderonth
Copy link
Copy Markdown

calderonth commented Aug 25, 2023

Right, I thought I could use this as a somewhat universal library to parse various lockfile format, so in top what you're doing you'd have to have edge cases for Yarn lockfiles which aren't JSON?

EDIT: I realise you have that logic backed into your code so I might get some inspiration from you :-)

@milahu
Copy link
Copy Markdown
Author

milahu commented Aug 25, 2023

yarn lockfiles are not-yet handled by my code

other limitations of snyk-nodejs-lockfile-parser:

@github-actions
Copy link
Copy Markdown

Your PR has not had any activity for 60 days. In 7 days I'll close it. Make some activity to remove this.

@github-actions github-actions Bot added the Stale label Jul 15, 2024
@github-actions
Copy link
Copy Markdown

Your PR has now been stale for 7 days. I'm closing it.

@github-actions github-actions Bot closed this Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants