Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(guided remediation): handle extraneous/missing packages in package-lock.json more leniently #1394

Merged
merged 4 commits into from
Nov 19, 2024

Conversation

michaelkedar
Copy link
Member

@michaelkedar michaelkedar commented Nov 12, 2024

Changes two things for package-lock.json parsing:

  1. Made it so packages installed in unknown locations (e.g. under a non-existent package) are ignored (following npm's behaviour), instead of triggering a panic.
  2. Made missing dependencies in the lockfile not cause an error. npm would typically raise an error here, unless the missing dependency under a workspace's dependency graph. I think it's ok if we're more lenient than npm here.

@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2024

Codecov Report

Attention: Patch coverage is 70.42254% with 21 lines in your changes missing coverage. Please review.

Project coverage is 68.88%. Comparing base (9a303ec) to head (3344c6f).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
internal/resolution/lockfile/npm_v2.go 66.66% 7 Missing and 3 partials ⚠️
internal/resolution/lockfile/npm_v1.go 70.00% 7 Missing and 2 partials ⚠️
internal/resolution/lockfile/npm.go 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1394      +/-   ##
==========================================
- Coverage   69.02%   68.88%   -0.14%     
==========================================
  Files         185      185              
  Lines       17869    17950      +81     
==========================================
+ Hits        12334    12365      +31     
- Misses       4876     4916      +40     
- Partials      659      669      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@michaelkedar michaelkedar changed the title fix(guided remediation): handle extraneous/missing packages in package-lock.json better fix(guided remediation): handle extraneous/missing packages in package-lock.json more leniently Nov 18, 2024
@michaelkedar michaelkedar marked this pull request as ready for review November 18, 2024 23:41
Children map[string]*npmNodeModule // keyed on package name
Deps map[string]string
OptionalDeps map[string]string
DevDeps map[string]string // dev dependencies are also included in Deps
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain a bit more why we don't need DevDeps and OptionalDeps here anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deps now also contains the dep.Type, which has whether it's optional or dev (or peer) in it.

@michaelkedar michaelkedar merged commit 8d59ca5 into google:main Nov 19, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants