feat: store package.json and deno.json JSR and npm deps in lockfile for tracking removable dependencies #13
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
…ckage.json, just in case
dsherret
changed the title
This was referenced Jan 20, 2024
… user to just delete it
bartlomieju
approved these changes
Jan 21, 2024
|
|
||
| /// Graph used to analyze a lockfile to determine which packages | ||
| /// and remotes can be removed based on config file changes. | ||
| pub struct LockfilePackageGraph<FNvToJsrUrl: Fn(&str) -> Option<String>> { |
There was a problem hiding this comment.
Huh, I didn't know you could do that (or never seen that in the wild)
Comment on lines
+139
to
+140
| while let Some(id) = pending.pop_back() { | ||
| if let Some(package) = packages.get_mut(&id) { |
There was a problem hiding this comment.
What if pacakges.get_mut() returns None? Is it assume it will be found by another root?
There was a problem hiding this comment.
It might occur if someone has been modifying the lockfile themselves. It's not a big deal in that case because then the package is already gone.
Comment on lines
+92
to
+104
| #[error("Integrity check failed for npm package: \"{package_display_id}\". Unable to verify that the package | ||
| is the same as when the lockfile was generated. | ||
|
|
||
| Actual: {actual} | ||
| Expected: {expected} | ||
|
|
||
| This could be caused by: | ||
| * the lock file may be corrupt | ||
| * the source itself may be corrupt | ||
|
|
||
| Use \"--lock-write\" flag to regenerate the lockfile at \"{filename}\".", | ||
| )] | ||
| pub struct IntegrityCheckFailedError { |
There was a problem hiding this comment.
Question: will we surface these errors one-by-one or aggregate them and display them all at once?
There was a problem hiding this comment.
One-by-one as it currently does. It might be good to improve this in the future.
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.
Updates the lockfile to keep track of JSR and npm dependencies stored in a deno.json or package.json. When a package is removed from a config file, then the dependency is also removed from the lockfile. This supports workspaces, so is nested under the "workspace" key:
A workspace with multiple members looks like this:
Additionally, JSR packages with their version constraints are now stored under "packages" in addition to "npm":
When
"jsr:@scope/package_a@1"is removed from "workspace" -> "dependencies", then it knows it can remove@scope/[email protected]and therefore@scope/[email protected](even though it's circular in this case) and@scope/[email protected]. Additionally, it strips out the urls under "remote". This specific example can be seen in https://github.com/denoland/deno_lockfile/pull/13/files#diff-e13153f86e029e465847ec0daa572a7d0e96acaea1ff47c232136f826208fb86