refactor: simplify dependenciesHierarchyForPackage by delegating to getTree#10616
refactor: simplify dependenciesHierarchyForPackage by delegating to getTree#10616
Conversation
…etTree Instead of manually iterating over top-level dependencies, calling getPkgInfo/getTreeNodeChildId/getTree per dependency, and handling dedup/search logic in parallel with materializeChildren, delegate entirely to a single getTree call with the importer as root. The returned PackageNode[] are then post-categorized into their dependency fields (dependencies, devDependencies, optionalDependencies) using a fieldMap built from the lockfile importer snapshot. This eliminates the duplicated dedup/search handling between dependenciesHierarchyForPackage and materializeChildren, and removes the GetTreeResult wrapper type from getTree (now returns PackageNode[] directly). The materializeChildren cache is now the sole mechanism for cross-importer deduplication. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors dependenciesHierarchyForPackage to delegate dependency tree building to a single getTree call using the importer as the root, rather than making separate calls for each top-level dependency. The returned nodes are then post-categorized into their dependency fields. This eliminates duplicated deduplication and search handling logic, simplifies the codebase, and removes the GetTreeResult wrapper type in favor of returning PackageNode[] directly.
Changes:
- Simplified
getTreeto returnPackageNode[]directly instead of aGetTreeResultwrapper - Refactored
dependenciesHierarchyForPackageto make a singlegetTreecall with the importer as root - Unified deduplication logic in
materializeChildrenas the sole cross-importer dedup mechanism
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| reviewing/dependencies-hierarchy/src/getTree.ts | Removed GetTreeResult interface and simplified getTree to return PackageNode[] directly, moving all dedup handling to materializeChildren |
| reviewing/dependencies-hierarchy/src/buildDependenciesHierarchy.ts | Refactored to call getTree once per importer with maxDepth: opts.depth + 1, then categorize nodes into dependency fields using a fieldMap; removed unused imports |
| reviewing/dependencies-hierarchy/test/getTree.test.ts | Updated tests to expect PackageNode[] return type and verify dedup behavior at node level instead of result wrapper level |
| cspell.json | Added "dedup" to dictionary |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (const field of DEPENDENCIES_FIELDS.sort().filter(f => opts.include[f])) { | ||
| result[field] = [] | ||
| const fieldDeps = currentLockfile.importers[importerId][field] ?? {} | ||
| for (const alias in fieldDeps) { | ||
| fieldMap.set(alias, field) | ||
| } | ||
| } |
There was a problem hiding this comment.
The filter condition opts.include[f] only includes fields where the value is truthy. However, in buildDependencyGraph, dependencies and devDependencies are included unless explicitly set to false (using !== false checks). This creates a mismatch: if opts.include.dependencies or opts.include.devDependencies is undefined, those dependencies will be included in the graph and returned by getTree, but they won't be added to fieldMap, causing nodes to be silently dropped during categorization. The filter should use opts.include[f] !== false to match the graph building logic.
…etTree (#10616) Instead of manually iterating over top-level dependencies, calling getPkgInfo/getTreeNodeChildId/getTree per dependency, and handling dedup/search logic in parallel with materializeChildren, delegate entirely to a single getTree call with the importer as root. The returned PackageNode[] are then post-categorized into their dependency fields (dependencies, devDependencies, optionalDependencies) using a fieldMap built from the lockfile importer snapshot. This eliminates the duplicated dedup/search handling between dependenciesHierarchyForPackage and materializeChildren, and removes the GetTreeResult wrapper type from getTree (now returns PackageNode[] directly). The materializeChildren cache is now the sole mechanism for cross-importer deduplication. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
`bit why` now shows a reverse dependency tree. The searched package appears at the root with its dependents as branches, walking back to workspace components. This replaces the previous forward-tree output which was noisy and hard to read for deeply nested dependencies. Also with the complete rewrite of the dependency tree builder we have fixed the out-of-memory errors that were frequent with the why command. The command also became a lot faster. ## Proposed Changes Related PRs: - pnpm/pnpm#10582 - pnpm/pnpm#10586 - pnpm/pnpm#10615 - pnpm/pnpm#10616 - pnpm/pnpm#10627 - pnpm/pnpm#10629 - pnpm/pnpm#10596 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Instead of manually iterating over top-level dependencies, calling getPkgInfo/getTreeNodeChildId/getTree per dependency, and handling dedup/search logic in parallel with materializeChildren, delegate entirely to a single getTree call with the importer as root.
The returned PackageNode[] are then post-categorized into their dependency fields (dependencies, devDependencies, optionalDependencies) using a fieldMap built from the lockfile importer snapshot.
This eliminates the duplicated dedup/search handling between dependenciesHierarchyForPackage and materializeChildren, and removes the GetTreeResult wrapper type from getTree (now returns PackageNode[] directly). The materializeChildren cache is now the sole mechanism for cross-importer deduplication.