-
Notifications
You must be signed in to change notification settings - Fork 243
ts-migration/no-if #324
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
ts-migration/no-if #324
Conversation
src/rules/no-if.ts
Outdated
| 'test.skip', | ||
| ]); | ||
|
|
||
| const isTestArrowFunction = (node: TSESTree.Node) => |
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.
This function is only used by ArrowFunctionExpression, so you can do node: TSESTree.ArrowFunctionExpression and ditch the undefined and node.type checks
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.
Actually apparently I can leave it as TSESTree.Node, but yeah much better!
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.
no need to widen the type when we don't need it 🙂
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.
Given the functions name (isTestArrowFunction), I think it should: either it should test that the type is ArrowFunctionExpression, or it should test if a given ArrowFunctionExpression is a "test" arrow function.
| } from '@typescript-eslint/experimental-utils'; | ||
|
|
||
| const isTestArrowFunction = node => | ||
| const testCaseNames = new Set<string | null>([ |
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.
null? seems wrong
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.
Yeah that's just b/c of how the typings work in relation to #has.
Basically, it's defined like this:
interface Set<T> {
has(item: T): boolean;
}
That means if T is just string, you can't do has checks against things like null, which is a possible return of getNodeName :)
Originally I had getNodeName(/* ... */) || '', but since that's a runtime solution, it's expected to be tested for by branch coverage 😂
src/rules/no-if.ts
Outdated
| const stack: Array<boolean> = []; | ||
|
|
||
| function validate(node) { | ||
| function validate(node: TSESTree.Node) { |
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.
| function validate(node: TSESTree.Node) { | |
| function validate(node: TSESTree.ConditionalExpression | TSESTree.IfStatement) { |
No description provided.