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
fix(eslint-plugin): [prefer-find] stop throwing type errors when converting symbols to numbers #8390
fix(eslint-plugin): [prefer-find] stop throwing type errors when converting symbols to numbers #8390
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,22 +125,22 @@ export default createRule({ | |
return isAtLeastOneArrayishComponent; | ||
} | ||
|
||
function getObjectIfArrayAtExpression( | ||
function getObjectIfArrayAtZeroExpression( | ||
node: TSESTree.CallExpression, | ||
): TSESTree.Expression | undefined { | ||
// .at() should take exactly one argument. | ||
if (node.arguments.length !== 1) { | ||
return undefined; | ||
} | ||
|
||
const atArgument = getStaticValue(node.arguments[0], globalScope); | ||
if (atArgument != null && isTreatedAsZeroByArrayAt(atArgument.value)) { | ||
const callee = node.callee; | ||
if ( | ||
callee.type === AST_NODE_TYPES.MemberExpression && | ||
!callee.optional && | ||
isStaticMemberAccessOfValue(callee, 'at', globalScope) | ||
) { | ||
const callee = node.callee; | ||
if ( | ||
callee.type === AST_NODE_TYPES.MemberExpression && | ||
!callee.optional && | ||
isStaticMemberAccessOfValue(callee, 'at', globalScope) | ||
) { | ||
const atArgument = getStaticValue(node.arguments[0], globalScope); | ||
if (atArgument != null && isTreatedAsZeroByArrayAt(atArgument.value)) { | ||
return callee.object; | ||
} | ||
} | ||
|
@@ -153,7 +153,14 @@ export default createRule({ | |
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/at#parameters | ||
*/ | ||
function isTreatedAsZeroByArrayAt(value: unknown): boolean { | ||
const asNumber = Number(value); | ||
let asNumber: number; | ||
|
||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Refactor] I'm not a big fan of Instead of generally catching all errors, how about checking if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, I was 50/50 on which way to go on this. Changed! |
||
asNumber = Number(value); | ||
} catch (e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is all that's strictly necessary for the fix
kirkwaiblinger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// This will happen if trying to convert a symbol. | ||
return false; | ||
} | ||
|
||
if (isNaN(asNumber)) { | ||
return true; | ||
|
@@ -215,7 +222,7 @@ export default createRule({ | |
return { | ||
// This query will be used to find things like `filteredResults.at(0)`. | ||
CallExpression(node): void { | ||
const object = getObjectIfArrayAtExpression(node); | ||
const object = getObjectIfArrayAtZeroExpression(node); | ||
if (object) { | ||
const filterExpression = parseIfArrayFilterExpression(object); | ||
if (filterExpression) { | ||
|
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 change just reverses the order of whether .at, or the (0), are inspected first. It's not needed for the fix, but I found it surprising that
foo(Symbol.for('throws'))
would throw, since it's not even anat
call; it's just any old function. WIth this change, you have to actually havex.at(Symbol.for('throws'))
in order to trigger the faulty code.