Repository navigation
Conversation
`bun pm ls` has no per-member output. When run from a workspace member it prints the root's dependencies, which are the members themselves (e.g. `app@workspace:app`), so every package was reported as "not a dependency" by `ng update`. When the listing shows the current package as a workspace entry, build the dependency map from the package's `package.json` and the manifests installed in `node_modules` (including the hoisted ones of parent directories) instead. Closes angular#34402
There was a problem hiding this comment.
Code Review
This pull request introduces a workaround for listing dependencies of a Bun workspace member, since bun pm ls does not support per-workspace-member output. It resolves this by manually reading the project's package.json and traversing node_modules to build the dependency map. The review feedback highlights a potential crash if package.json parses to a non-object value (such as null or a primitive), which would cause a TypeError when accessing dependency fields. It is recommended to add defensive checks to ensure declared and its dependency fields are valid objects before extracting keys.
| let declared: Record<string, Record<string, string> | undefined>; | ||
| try { | ||
| declared = JSON.parse(await this.host.readFile(join(this.cwd, 'package.json'))); | ||
| } catch { | ||
| return dependencies; | ||
| } | ||
|
|
||
| const names = new Set<string>(); | ||
| for (const field of ['dependencies', 'devDependencies', 'optionalDependencies']) { | ||
| Object.keys(declared[field] ?? {}).forEach((name) => names.add(name)); | ||
| } |
There was a problem hiding this comment.
If package.json contains null or a non-object value (e.g., "null"), JSON.parse will succeed but declared will be null or a primitive. Accessing declared[field] will then throw a TypeError outside of the try-catch block, crashing the CLI.
Additionally, if a dependency field is malformed (e.g., a string or array instead of an object), Object.keys might return unexpected keys (like array indices or string character offsets).
We should add defensive checks to ensure declared is a non-null object and that each dependency field is also a valid object before extracting keys.
let declared: unknown;
try {
declared = JSON.parse(await this.host.readFile(join(this.cwd, 'package.json')));
} catch {
return dependencies;
}
if (!declared || typeof declared !== 'object' || Array.isArray(declared)) {
return dependencies;
}
const names = new Set<string>();
for (const field of ['dependencies', 'devDependencies', 'optionalDependencies']) {
const deps = (declared as Record<string, unknown>)[field];
if (deps && typeof deps === 'object' && !Array.isArray(deps)) {
Object.keys(deps).forEach((name) => names.add(name));
}
}
PR Checklist
PR Type
What is the current behavior?
In a bun workspace where the Angular project is a member,
ng update @angular/corereports "Package '@angular/core' is not a dependency", becausebun pm lsrun from a member prints the root's dependencies, i.e. the members (app@workspace:app).Issue Number: #34402
What is the new behavior?
When the listing shows the current package as a
workspace:entry, the dependency map is built from the package'spackage.jsonand the installed manifests innode_modules, including hoisted parent directories. I only ran the new spec inpackage-manager_spec.ts(fails on base, passes on this commit), not the full test suite.Does this PR introduce a breaking change?
Other information