-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(eslint-plugin): [no-type-alias] support tuples #775
feat(eslint-plugin): [no-type-alias] support tuples #775
Conversation
Thanks for the PR, @ankeetmaini! 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 |
24edd61
to
9cf765c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea but not quite right.
the reason that the cases failed is because we never added handling for TSTupleType
.
What we will want to do instead is:
- add back the undhandled case error.
- add a new option
allowTupleTypes
, which follows the same schema asallowMappedTypes
. - add in a new clause in
validateTypeAliases
which handlesTSTupleType
Thanks @bradzacher, updated the PR :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome! code lgtm. nice work covering everything well.
thanks for doing this.
Can we also have a test for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's actually an interesting point - this rule doesn't support TSTypeOperator
at all.
Considering the existence of readonly [tuple]
, this should also add support for that.
Wouldn't be a huge change to also support the keyof
operator too.
if (type.node.type === AST_NODE_TYPES.TSTypeOperator) {
if (
(['keyof', 'readonly'].includes(type.node.operator) &&
type.node.typeAnnotation &&
type.node.typeAnnotation.type === 'TSTupleType'
) {
return true;
}
}
TS is throwing on this line How else can I ascertain that |
try |
It fails with that too. :( |
It's part of the type union, so it should work: |
The types were wrong for |
29d8f5d
to
8874a3e
Compare
@bradzacher thanks, I've updated the tests and rebased with master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for doing this
Fixes #720