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
[DO NOT MERGE] Auto fix prefer-destructuring-in-parameters
on codebase
#1046
[DO NOT MERGE] Auto fix prefer-destructuring-in-parameters
on codebase
#1046
Conversation
@@ -98,8 +98,8 @@ const create = context => { | |||
const member = source.getText(node.property); | |||
|
|||
// Member might already be destructured | |||
const destructuredMember = destructurings.find(property => | |||
property.key.name === member | |||
const destructuredMember = destructurings.find(({key}) => |
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.
The problem with destructuring is that you lose some readability information. It's no longer clear when you read this that key
is the key of a property...
scope.block && | ||
scope.block.type === 'ArrowFunctionExpression' && | ||
(scope.thisFound || scope.childScopes.some(scope => isArrowFunctionWithThis(scope))); | ||
const isReactHook = ({block}) => |
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.
It's no longer possible to infer from the function what type it supports (that it supports a scope...
const isArrowFunctionWithThis = ({type, block, thisFound, childScopes}) => | ||
type === 'function' && | ||
block && | ||
block.type === 'ArrowFunctionExpression' && | ||
(thisFound || childScopes.some(scope => isArrowFunctionWithThis(scope))); |
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 think the previous version was more readable, and not much longer.
Gonna close this for now as it's uncertain what we'll do with this rule. |
Changes a lot, I'd like to fix codebase after #1045 get merged.