Report qs instead of @types/qs for extraneous deps#3258
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes no-extraneous-dependencies so that when an import resolves only via an @types/* devDependency, the rule reports the imported runtime package name (e.g. qs) rather than the @types/* package.
Changes:
- Adjust rule reporting to prefer the imported package name when the resolver lands on an
@types/*package. - Add a regression test fixture and RuleTester case for the
@types-only resolution scenario (issue #3208). - Document the fix in the changelog and add missing reference links/credits.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/rules/no-extraneous-dependencies.js | Adds a regression test ensuring the error mentions the runtime package instead of @types/*. |
| tests/files/with-resolved-types-dependencies/package.json | Adds a fixture package.json containing only an @types/* devDependency. |
| tests/files/with-resolved-types-dependencies/foo.js | Adds a fixture file used as the test filename target. |
| src/rules/no-extraneous-dependencies.js | Updates reporting logic to use the imported name when resolution is through @types/*. |
| CHANGELOG.md | Notes the fix and adds associated links/attributions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3258 +/- ##
==========================================
- Coverage 95.68% 95.45% -0.24%
==========================================
Files 83 83
Lines 3732 3737 +5
Branches 1351 1352 +1
==========================================
- Hits 3571 3567 -4
- Misses 161 170 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // when the import only resolves through an `@types/*` package (e.g. importing | ||
| // `qs` resolves to `@types/qs`), the missing runtime package is the imported | ||
| // name, not the `@types/*` package the resolver landed on | ||
| const reportedPackageName = realPackageName && realPackageName.startsWith('@types/') |
There was a problem hiding this comment.
For the @types/* case this reports importPackageName. That's correct for direct imports, but it leans on the imported name - and the comment just above notes importPackageName "can be misinterpreted" for aliased imports (an alias resolving into @types/qs would report the alias, not qs).
The runtime name is deterministically derivable from realPackageName via DefinitelyTyped's scope-mangling formula - the same transform TypeScript itself uses (getPackageNameFromTypesPackageName → unmangleScopedPackageName in src/compiler/moduleNameResolver.ts): @types/qs → qs, @types/babel__core → @babel/core.
// mirrors TS's unmangleScopedPackageName
function runtimePackageFromTypes(name) {
const inner = name.slice('@types/'.length);
return inner.includes('__') ? `@${inner.replace('__', '/')}` : inner;
}@definitelytyped/utils exports this as unmangleScopedPackage, but that dep's engines range is too restrictive. Deriving from realPackageName handles scoped @types packages and aliased imports robustly, rather than trusting the imported string.
| // name, not the `@types/*` package the resolver landed on | ||
| const reportedPackageName = realPackageName && realPackageName.startsWith('@types/') | ||
| ? importPackageName | ||
| : realPackageName || importPackageName; |
There was a problem hiding this comment.
This || importPackageName fallback is the codecov/project failure: it's the one uncovered branch ([34,0] - realPackageName is always truthy in the suite, so the right operand never runs), which drops branch coverage below base. Adopting the realPackageName-derived formula above removes this fallback entirely; otherwise a test exercising a falsy realPackageName is needed to cover it.
…`@types/*`, when an import resolves via a `@types` devDependency
16ece0c to
895fd8a
Compare
no-extraneous-dependenciescould point at@types/qseven though the package a user needs to declare isqs.When resolution lands in an
@types/*package, the report now uses the runtime package name.Checked it with the focused mocha run, the red/green case, eslint, and markdownlint.
Closes #3208