Skip to content

Commit e95f620

Browse files
elrrrrrrrclaude
andauthored
refactor(pm): use typed PackageLock in outdated check (#2674)
Replace raw `serde_json::Value` access with typed `PackageLock` and `LockPackage` struct fields in `is_pkg_lock_outdated`. Simplify `deps_map_equals_lock` from 23 lines to 6 by accepting typed `Option<&HashMap>` directly. Simplify engines comparison via `PartialEq`. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e994e4a commit e95f620

1 file changed

Lines changed: 21 additions & 44 deletions

File tree

crates/pm/src/helper/lock.rs

Lines changed: 21 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -53,28 +53,16 @@ pub fn extract_package_name(path: &str) -> String {
5353
}
5454
}
5555

56-
/// Compare a PackageJson dependency map with a lock file Value field.
56+
/// Compare a PackageJson dependency map with a lock file dependency map.
5757
/// Treats empty maps and None/empty objects as equal.
58-
fn deps_map_equals_lock(pkg_deps: &HashMap<String, String>, lock_field: Option<&Value>) -> bool {
59-
// Convert lock field to HashMap for comparison
60-
let lock_deps: HashMap<String, String> = match lock_field {
61-
Some(val) => val
62-
.as_object()
63-
.map(|obj| {
64-
obj.iter()
65-
.filter_map(|(k, v)| v.as_str().map(|s| (k.clone(), s.to_string())))
66-
.collect()
67-
})
68-
.unwrap_or_default(),
69-
None => HashMap::new(),
70-
};
71-
72-
// Treat empty maps as equal
73-
if pkg_deps.is_empty() && lock_deps.is_empty() {
74-
return true;
58+
fn deps_map_equals_lock(
59+
pkg_deps: &HashMap<String, String>,
60+
lock_deps: Option<&HashMap<String, String>>,
61+
) -> bool {
62+
match lock_deps {
63+
Some(ld) => *pkg_deps == *ld,
64+
None => pkg_deps.is_empty(),
7565
}
76-
77-
*pkg_deps == lock_deps
7866
}
7967

8068
pub async fn ensure_package_lock(root_path: &Path) -> Result<PackageLock> {
@@ -372,13 +360,9 @@ pub async fn is_pkg_lock_outdated(root_path: &Path) -> Result<bool> {
372360
// package.json has diverged from the lock file, so it must not use the
373361
// in-process cache.
374362
let pkg: DepsView = load_package_json(root_path).await?;
375-
let lock_file: Value = read_json_file(&root_path.join("package-lock.json")).await?;
363+
let lock_file: PackageLock = read_json_file(&root_path.join("package-lock.json")).await?;
376364

377-
// get packages in package-lock.json
378-
let packages = lock_file
379-
.get("packages")
380-
.and_then(|p| p.as_object())
381-
.ok_or_else(|| anyhow!("Invalid package-lock.json format"))?;
365+
let packages = &lock_file.packages;
382366

383367
// prepare packages to check: (relative_path, deps)
384368
let mut pkgs_to_check: Vec<(String, DepsView)> = vec![("".to_string(), pkg)];
@@ -407,29 +391,30 @@ pub async fn is_pkg_lock_outdated(root_path: &Path) -> Result<bool> {
407391
}
408392
};
409393

394+
let name = if path.is_empty() { "root" } else { path };
395+
410396
// check dependencies whether changed
411-
if !deps_map_equals_lock(&pkg.dependencies, lock.get("dependencies")) {
412-
let name = if path.is_empty() { "root" } else { path };
397+
if !deps_map_equals_lock(&pkg.dependencies, lock.dependencies.as_ref()) {
413398
tracing::warn!("package-lock.json is outdated, {name} dependencies changed");
414399
return Ok(true);
415400
}
416401

417-
if !deps_map_equals_lock(&pkg.optional_dependencies, lock.get("optionalDependencies")) {
418-
let name = if path.is_empty() { "root" } else { path };
402+
if !deps_map_equals_lock(
403+
&pkg.optional_dependencies,
404+
lock.optional_dependencies.as_ref(),
405+
) {
419406
tracing::warn!("package-lock.json is outdated, {name} optionalDependencies changed");
420407
return Ok(true);
421408
}
422409

423-
if !deps_map_equals_lock(&pkg.dev_dependencies, lock.get("devDependencies")) {
424-
let name = if path.is_empty() { "root" } else { path };
410+
if !deps_map_equals_lock(&pkg.dev_dependencies, lock.dev_dependencies.as_ref()) {
425411
tracing::warn!("package-lock.json is outdated, {name} devDependencies changed");
426412
return Ok(true);
427413
}
428414

429415
if !legacy_peer_deps
430-
&& !deps_map_equals_lock(&pkg.peer_dependencies, lock.get("peerDependencies"))
416+
&& !deps_map_equals_lock(&pkg.peer_dependencies, lock.peer_dependencies.as_ref())
431417
{
432-
let name = if path.is_empty() { "root" } else { path };
433418
tracing::warn!("package-lock.json is outdated, {name} peerDependencies changed");
434419
return Ok(true);
435420
}
@@ -442,19 +427,11 @@ pub async fn is_pkg_lock_outdated(root_path: &Path) -> Result<bool> {
442427
.ok_or_else(|| anyhow!("Missing root in package-lock.json"))?;
443428

444429
let pkg_engines = root_engines.engines.as_ref().filter(|m| !m.is_empty());
445-
let lock_engines = root_lock
446-
.get("engines")
447-
.filter(|v| !v.is_null())
448-
.and_then(|v| v.as_object())
449-
.filter(|obj| !obj.is_empty());
430+
let lock_engines = root_lock.engines.as_ref().filter(|m| !m.is_empty());
450431

451432
let engines_match = match (pkg_engines, lock_engines) {
452433
(None, None) => true,
453-
(Some(p), Some(l)) => {
454-
p.len() == l.len()
455-
&& p.iter()
456-
.all(|(k, v)| l.get(k).and_then(|x| x.as_str()) == Some(v.as_str()))
457-
}
434+
(Some(p), Some(l)) => *p == *l,
458435
_ => false,
459436
};
460437

0 commit comments

Comments
 (0)