Skip to content

fix(lockfile): gracefully skip empty package names in package.json dependencies#33094

Open
wdskuki wants to merge 1 commit intodenoland:mainfrom
wdskuki:fix/lockfile-empty-package-name
Open

fix(lockfile): gracefully skip empty package names in package.json dependencies#33094
wdskuki wants to merge 1 commit intodenoland:mainfrom
wdskuki:fix/lockfile-empty-package-name

Conversation

@wdskuki
Copy link
Copy Markdown

@wdskuki wdskuki commented Mar 31, 2026

This fixes an issue where lockfile deserialization fails when package.json contains empty package names (e.g., {"": "."}), which can occur from recursive npm installs.

Instead of failing the entire deserialization, invalid entries are now silently skipped, allowing the lockfile to load successfully.

Fixes: #32113

Testing:

  • All existing lockfile tests pass
  • Manually verified with a package.json containing empty dependency name

Changes:

  • Added custom deserializer deserialize_package_json_deps that skips invalid entries
  • Empty package names and unparseable dependencies are gracefully ignored

…pendencies

This fixes an issue where lockfile deserialization fails when package.json
contains empty package names (e.g., {"": "."}), which can occur from
recursive npm installs.

Instead of failing the entire deserialization, invalid entries are now
silently skipped, allowing the lockfile to load successfully.

Fixes denoland#32113
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment on lines +31 to +34
// Should not fail to deserialize - empty package names are skipped
// Note: This is a compile-time deserialization test
// The actual runtime behavior depends on JsrDepPackageReq parsing
let _content = lockfile_content; // Placeholder - actual test requires async setup
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like AI slop with unfinished test

Copy link
Copy Markdown
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

Did you use AI to create this PR?

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.

3 participants