fix(type-utils): handle external packages that are not in program.sourceFileToPackageName
#10097
Conversation
|
Thanks for the PR, @abrahamguo! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 3c2ffaa. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10097 +/- ##
==========================================
- Coverage 86.57% 86.09% -0.48%
==========================================
Files 431 428 -3
Lines 15198 14970 -228
Branches 4421 4343 -78
==========================================
- Hits 13158 12889 -269
- Misses 1683 1734 +51
+ Partials 357 347 -10
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| const packageNameMatcher = new RegExp(`${packageName}|${typesPackageName}`); | ||
| const fileNameMatcher = new RegExp( | ||
| `node_modules/(?:${packageName}|@types/${typesPackageName})/`, | ||
| ); |
There was a problem hiding this comment.
https://www.typescriptlang.org/tsconfig/#typeRoots - if someone customizes this, then fileNameMatcher will be wrong. This'll need to reflect the project's compilerOptions.typeRoots.
There was a problem hiding this comment.
(moved to https://github.com/typescript-eslint/typescript-eslint/pull/10097/files#r1854146022 to be closer to implementation)
|
👋 Just checking in @abrahamguo, anything we can help with? |
|
Should be ready for review now, sorry for the delay! The remaining CI failures come from |
| const packageNameRegExp = escapeAlternates([ | ||
| packageName, | ||
| // Handle scoped packages: if the name starts with @, remove it and replace / with __ | ||
| `@types/${packageName.replace(/^@([^/]+)\//, '$1__')}`, |
There was a problem hiding this comment.
[Bug] @types/ should not be hardcoded to always be considered. From https://www.typescriptlang.org/tsconfig/#typeRoots:
If
typeRootsis specified, only packages undertypeRootswill be included.
|
👋 Hey @abrahamguo! Just checking in, is this still something you have time for? No worries if not - I just don't want to leave it hanging. |
|
No, unfortunately I don't have time at the moment to finish this up! |
|
👍 No problem, thanks for getting it started! Closing out then. If anybody wants to continue this work, please do! And if it builds off this PR, please add a co-author attribution. |
PR Checklist
Buffercannot be matched with a{ type: package }TypeOrValueSpecifier#10089Overview
.d.tsfiles are not included in TypeScript'sprogram.sourceFileToPackageNamemap.TypeOrValueSpecifier, it looked like all of the@types/nodeand@types/lodashfiles except forindex.d.tswere missing fromprogram.sourceFileToPackageName.{ from: 'package' }TypeOrValueSpecifiers.program.sourceFileToPackageNamewas introduced in Bug: v6typeMatchesSpecifierPackageSpecifiershould match custom type roots #6868 => fix(type-utils): treat custom type roots as external #6870.program.sourceFileToPackageName. I cannot see any issues that would arise from doing this.