Skip to content
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

Add 'consistent-destructuring' exceptions #1501

Closed
Nantris opened this issue Aug 25, 2021 · 4 comments
Closed

Add 'consistent-destructuring' exceptions #1501

Nantris opened this issue Aug 25, 2021 · 4 comments

Comments

@Nantris
Copy link

Nantris commented Aug 25, 2021

I've found that destructuring React's state and props off of this can be a bit unwieldy due to the depth of destructuring. It would be great to be able to exclude select patterns - in our particular case, this.state and this.props.

    const { someMethod } = this;
    const { someBool } = this.state; // <<< unicorn/consistent-destructuring flags 'this.state'

This is pretty trivial as we can just move the state/props destructures above like this, but it would still be great if this was supported.

    const { someBool } = this.state;
    const { someMethod } = this;

Maybe it's just me, but I find destructuring directly from this.state to be easier to reason about than first destructuring state off of this.


It's also a bit of a pain when refactoring if you're getting rid of someMethod and you have the following:

const { someMethod, state } = this
const { someBool } = state;

because now you're left with this:

const { state } = this;
const { someBool } = state;

That destructuring assignment seems needless at that point, but to remove it requires updating two lines of code, which can be tedious during refactors. Even if each individual case only adds a couple seconds, it adds up in terms of time and reducing focus on the task at hand.

@futpib
Copy link
Contributor

futpib commented Aug 30, 2021

If someMethod is indeed a method isn't it better to call it like this.someMethod()?

@Nantris
Copy link
Author

Nantris commented Aug 30, 2021

That's safer, and my general preference, but sometimes it makes sense (in my opinion) to destructure it off if you're passing it down to a child component under the same name.

@RettentoRectangle
Copy link

RettentoRectangle commented Nov 19, 2021

Exceptions would be welcome!

In customElements there are many times we had to disable this rule because of e.g.: classList.

Example:

const { foo, bar, baz } = this
// ... later in that function
this.classList.toggle(styles.empty) // <--- Use destructured variables over properties.eslintunicorn/consistent-destructuring)

@fregante
Copy link
Collaborator

The rule was recently removed from the "recommended" config and this issue hasn't seen activity in a while. Closing for now

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants