Parse lockfile recursively #5
Conversation
lib/index.ts
Outdated
| return depSubTree; | ||
| } else { | ||
| // no more deps, return tree | ||
| return depSubTree; |
There was a problem hiding this comment.
don't think you need an else here since you return in the statement above
There was a problem hiding this comment.
You are right, I'll move return to the separate statement.
lib/index.ts
Outdated
| } else { | ||
| // tree was walked to the root and dependency was not found | ||
| if (!depKeys.length) { | ||
| throw new Error(`Dependency ${dep} was not found in package-lock.json.`); |
There was a problem hiding this comment.
Can we make this message more verbose and let the user know that it looks like their package.json and their lockfile are out of sync and give them some advice how to sync them up?
lib/index.ts
Outdated
| if (deps[dep].requires) { | ||
| Object.keys(deps[dep].requires).forEach(async (subDep) => { | ||
| depSubTree.dependencies[subDep] = await buildSubTreeRecursive(subDep, [...depKeys, subDep]); | ||
|
|
There was a problem hiding this comment.
Can you run the linter on this, looks like they might be some linting fixes to add.
There was a problem hiding this comment.
tslint doesn't complain.
There was a problem hiding this comment.
Didn't complain :-D It does now.
package.json
Outdated
| @@ -25,11 +25,12 @@ | |||
| "devDependencies": { | |||
| "@types/node": "10.5.5", | |||
| "@types/sinon": "5.0.1", | |||
| "semantic-release": "^8.2.0", | |||
miiila
force-pushed
the
feat/parse-lockfile-recursively
branch
2 times, most recently
from
c1b2b33
to
df0a59e
Compare
miiila
dismissed
lili2311’s stale review
All comments addressed, thank you for them.
miiila
force-pushed
the
feat/parse-lockfile-recursively
branch
from
df0a59e
to
3181788
Compare
miiila
force-pushed
the
feat/parse-lockfile-recursively
branch
from
3181788
to
69290b1
Compare
miiila
force-pushed
the
feat/parse-lockfile-recursively
branch
from
f10b991
to
e48b713
Compare
bin/index.js
Outdated
| .then((tree) => { | ||
| console.log(JSON.stringify(tree)); | ||
| }) | ||
| .catch((e) => { |
There was a problem hiding this comment.
.catch(console.error); might be better here
package.json
Outdated
| @@ -2,6 +2,9 @@ | |||
| "name": "snyk-nodejs-lockfile-parser", | |||
| "description": "Generate a dep tree given a lockfile", | |||
| "main": "dist/lib/index.js", | |||
| "bin": { | |||
| "parse": "./bin/index.js" | |||
There was a problem hiding this comment.
I think "parse" is probably too generic for a bin that will be installed globally
There was a problem hiding this comment.
Not sure what a better name might be.. maybe "parse-npm-lockfile"??
lib/index.ts
Outdated
| if (!root || !lockFilePath || !lockFilePath) { | ||
| throw new Error('Missing required parameters for parseLockFile()'); | ||
| } | ||
| // TODO: validate only valid options were passed in |
miiila
force-pushed
the
feat/parse-lockfile-recursively
branch
from
2eb170c
to
c04aeee
Compare
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What this does
Introduces a recursive algorithm for creation of dependency tree from
package-lock.jsonandpackage.json.Notes for the reviewer
I did my best to comment the algorithm, but feel free to suggest any naming or other structural improvements.
More information