fix(npm): add support for npm packages in lock files #15938
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dsherret
reviewed
Sep 17, 2022
|
Discussed offline with @dsherret. This PR needs more work as we need to store way more information in the lock file. After some discussion we are in agreement that we need a new format for the config file, which is a much bigger endeavor and will probably be done in v1.27. |
bartlomieju
commented
Oct 21, 2022
dsherret
reviewed
Oct 21, 2022
bartlomieju
commented
Oct 24, 2022
dsherret
reviewed
Oct 25, 2022
cli/lockfile.rs
Outdated
| .unwrap_or(&package.dist.shasum); | ||
| if &package_info.integrity != integrity { | ||
| return Err(anyhow!( | ||
| "Integrity check failed for npm package: {}. Pass --lock-write to update the lockfile.", |
There was a problem hiding this comment.
Do other tools recommend to update the lockfile if the integrity check fails?
There was a problem hiding this comment.
Example from yarn:
Incorrect integrity when fetching from the cache for "vite". Cache has "sha512-m7jJe3nufUbuOfotkntGFupinL/fmuTNuQmiVE7cH2IZMuf4UbfbGYMUT3jVWgGYuRVLY9j8NnrRqgw5rr5QTg== sha1-+ikUQWfRm3c7r/1ls5cupMEjWck=" and remote has "sha512-asdfasdf". Run `yarn cache clean` to fix the problem
dsherret
reviewed
Oct 25, 2022
dsherret
reviewed
Oct 25, 2022
dsherret
approved these changes
Oct 25, 2022
cli/lockfile.rs
Outdated
| Resolved to \"{}@{}\" and lockfile expects \"{}\". | ||
| Use \"--lock-write\" flag to update the lockfile.", | ||
| package_req.name, package_req.name, version, resolved_specifier | ||
| ))); |
There was a problem hiding this comment.
I'm not sure this would ever occur because the resolution should be informed by the lockfile and it seems there's no test for this. Maybe we should remove this function?
There was a problem hiding this comment.
Okay, I'm going to remove it then.
dsherret
reviewed
Oct 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #15696