feat: detect phantom dependencies#753
Conversation
|
Hey @goureeshreddy7 - this branch has merge conflicts against main. Could you rebase and resolve them, then force-push? Happy to review once it's up to date. |
|
Hey @goureeshreddy7, the branch has merge conflicts that need to be resolved before I can review. Could you rebase against main and push an update? |
goureeshreddy7
force-pushed
the
feat/phantom-dependencies
branch
from
2011b8e to
e056bc4
Compare
|
@sonukapoor sir, I have successfully rebased the branch against main and resolved the merge conflicts as you said, Let me know if anything to doo ! |
sonukapoor
left a comment
sonukapoor
left a comment
There was a problem hiding this comment.
One blocker at the file level: pr592_comments.json needs to be removed before merge. It looks like a working note that was accidentally staged - it will end up in the npm tarball and clutter the repo permanently once merged.
| } | ||
|
|
||
| if (options.usage) { | ||
| if (scanState.pd001 && scanState.pd001.length > 0) { |
There was a problem hiding this comment.
Phantom detection is gated on options.usage, but --usage is a separate feature for import-reachability filtering with its own semantics. Users who don't pass --usage get no phantom detection at all, and users who do pass --usage get phantom detection they didn't ask for. These should be decoupled - either run phantom detection unconditionally or add a dedicated --check-phantoms flag.
| console.log(chalk.yellow("\n[PD002] Transitive-only Phantom Dependencies Detected")); | ||
| scanState.pd002.forEach(pkg => console.log(` - ${pkg}`)); | ||
| } | ||
| if ((scanState.pd001 && scanState.pd001.length > 0) || (scanState.pd002 && scanState.pd002.length > 0)) { |
There was a problem hiding this comment.
Phantom findings aren't wired into --json output - machine consumers parsing JSON (CI pipelines, dashboards) get no phantom data at all. The pd001/pd002 results need to be included in the JSON schema before this feature is usable in CI.
| scanState.sorted.some(reachesFailOn) || overrideFindings.some(reachesFailOn); | ||
| scanState.sorted.some(reachesFailOn) || | ||
| overrideFindings.some(reachesFailOn) || | ||
| (options.usage && ((scanState.pd001 && scanState.pd001.length > 0) || (scanState.pd002 && scanState.pd002.length > 0))); |
There was a problem hiding this comment.
Any phantom finding triggers exit code 1 here regardless of what --fail-on is set to. A project running --fail-on critical would fail hard on any phantom import, including low-risk dev tooling. This silently breaks the CI contract --fail-on is supposed to provide.
| } | ||
| } | ||
|
|
||
| export function readOverridesAndResolutions(projectPath: string): Set<string> { |
There was a problem hiding this comment.
The existing hasOverrideEntries just above this (line 151-153) correctly checks pnpm.overrides nested under the pnpm key. readOverridesAndResolutions only reads top-level overrides and resolutions, so pnpm projects using pnpm.overrides will have their overridden packages misclassified as pd002 instead of pd001.
2 participants
Fixes #731
What does this PR do? Implements the Phantom Dependency detection engine to catch unlisted imports that break strict environments like Vercel deployments (PD001 and PD002).
Key Changes:
AST Import Scanner: Upgraded scanProjectForPackageUsage in src/usage/scanner.ts to capture all bare module imports across the source code rather than just a pre-filtered list.
Overrides parsing: Added readOverridesAndResolutions to src/utils/package-json.ts to properly parse the overrides and resolutions blocks.
Cross-Referencing: Updated src/index.ts to cross-reference the import map against the declared dependencies.
Ranked Output: Output is ranked by build impact as requested, with strict deploy-breakers (PD001 - override pins) surfaced above standard transitive ghosts (PD002).
Testing: Verified locally that the CLI correctly catches ghost dependencies and exits with an error code, effectively stopping the build before deployment.