-
Notifications
You must be signed in to change notification settings - Fork 248
fix(unbound-method): fix issue 1800 #1862
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
base: main
Are you sure you want to change the base?
Conversation
G-Rath
left a comment
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.
at this point I feel like this is an overly complicated solution - have you looked into why our existing logic for handling jest.mocked isn't catching this case?
Sorry for the response, in this case the |
G-Rath
left a comment
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.
I still would expect we can identify the call with parseJestFn, but what you've got it a lot simpler than what you had before and finding the right node to parse can be a pain at times, so I'm happy to go with this once you've simplified it a bit more with the isSupportedAccessor change
| // Check if the call is jest.mocked() by examining the callee | ||
| if ( | ||
| parentCall.callee.type === AST_NODE_TYPES.MemberExpression && | ||
| isSupportedAccessor(parentCall.callee.object) && |
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.
isSupportedAccessor takes an optional value as its second param for just these situations - so rather than having to do the work twice, you can just do isSupportedAccessor(..., 'jest')
that in turn means you should be able to refactor this all to be a return
|
|
||
| // Also try using parseJestFnCall as a fallback | ||
| // const jestFnCall = parseJestFnCall( | ||
| // findTopMostCallExpression(parentCall), | ||
| // context, | ||
| // ); | ||
|
|
||
| // return ( | ||
| // jestFnCall?.type === 'jest' && | ||
| // jestFnCall.members.length >= 1 && | ||
| // isIdentifier(jestFnCall.members[0], 'mocked') | ||
| // ); |
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.
| // Also try using parseJestFnCall as a fallback | |
| // const jestFnCall = parseJestFnCall( | |
| // findTopMostCallExpression(parentCall), | |
| // context, | |
| // ); | |
| // return ( | |
| // jestFnCall?.type === 'jest' && | |
| // jestFnCall.members.length >= 1 && | |
| // isIdentifier(jestFnCall.members[0], 'mocked') | |
| // ); |
| node: TSESTree.MemberExpression, | ||
| ): boolean => { | ||
| // Check if the immediate parent is a CallExpression | ||
| if (node.parent?.type !== AST_NODE_TYPES.CallExpression) { |
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 is the opposite of the check we do later on, so ideally we can merge the two but that might be messy due to the optional chaining and require an else which I try to avoid so not too fussed to keep it like this for now
Fix: #1800