fix(type-utils): allow FileSpecifier to exclude paths
#6860
Conversation
…luding node_modules
|
Thanks for the PR, @RebeccaStevens! 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 settings. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## v6 #6860 +/- ##
==========================================
- Coverage 87.46% 87.45% -0.02%
==========================================
Files 374 374
Lines 12879 12884 +5
Branches 3811 3814 +3
==========================================
+ Hits 11265 11268 +3
- Misses 1229 1230 +1
- Partials 385 386 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Thanks for the PR, you truly beat me to it by just a few minutes :D
As I wrote in the comments, I'm not a fan of the paths being configurable and would just hard-code node_modules there.
However, if you do decide to go ahead with the excludePaths option, typeOrValueSpecifierSchema needs to be modified as well :)
|
I'm going to wait on discussion in #6839 - it might impact the config option here. |
|
This was unintentionally auto-closed when we merged the |
|
Sorry for the delay - #6839 is accepting PRs so this is unblocked! |
| path?: undefined; | ||
| excludePaths?: string | string[]; // defaults to `["node_modules"]`. | ||
| } | ||
| ); |
There was a problem hiding this comment.
Starting a new conversation here: why make excludePaths and path mutually exclusive? What if someone wants to provide both? E.g.:
{
path: "examples",
excludePaths: ["examples/*/node_modules"]
}Proposal: can we switch this back to a straightforward interface?
interface FileSpecifier {
from: 'file';
name: string | string[];
path?: string;
excludePaths?: string | string[]; // defaults to `["node_modules"]`.
}| } | ||
| | { | ||
| path?: undefined; | ||
| excludePaths?: string | string[]; // defaults to `["node_modules"]`. |
There was a problem hiding this comment.
I also wonder if we should call this pathExcludes, to visually lump it in with path... 🤔
|
👋 ping @RebeccaStevens, just checking in - is this still something you have time for? No worries if not! I just don't want to leave it hanging. |
|
Closing this PR as it's been stale for a while without activity. Feel free to reopen @RebeccaStevens if you have time - but no worries if not! If anybody wants to drive it forward, please do post your own PR - and if you use this as a start, consider adding a co-author at the end of your PR description. Thanks! 😊 |
PR Checklist
typeMatchesSpecifierFileSpecifierchecks in node modules #6819Overview
Adds
excludePathsoption toFileSpecifier(has no effect if apathis given).This new option defaults to
["node_modules"].