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

no-unused-vars option to allow unused vars in object destructuring with rest property #4880

Closed
bryanrsmith opened this issue Jan 7, 2016 · 15 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion triage An ESLint team member will look at this issue soon

Comments

@bryanrsmith
Copy link
Contributor

I'd like an option for the no-unused-vars rule that would allow unused variables that are created as part of an object destructuring pattern that also includes an experimental rest property.

Use case: experimental object rest properties offer a convenient way to shallow clone an object while omitting certain properties.
For example,

let foo = { a: 1, b: 2, extra: { other: 'stuff' }};

// I want a shallow clone of `foo` that omits `extra`
let { extra, ...bar } = foo; // error-- `extra` is unused

return bar; // { a: 1, b: 2 }

This can be done now by naming the unused variables with a pattern (instead of using the shorthand) and configuring no-unused-vars with varsIgnorePattern. It's less convenient and it adds noise, though.

let { extra: ignored, ...bar } = foo;

Rest/spread properties are still a stage 2 proposal, so it might be premature to add an option to a core rule. I figured I'd open for discussion anyways :-)

@eslintbot
Copy link

@bryanrsmith Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 7, 2016
@nzakas
Copy link
Member

nzakas commented Jan 8, 2016

Yeah, too early to add an exception. We don't make special affordances for experimental features, but feel free to follow up if it becomes official.

@nzakas nzakas closed this as completed Jan 8, 2016
@bryanrsmith
Copy link
Contributor Author

Thanks. Will do.
On Thu, Jan 7, 2016 at 5:28 PM Nicholas C. Zakas notifications@github.com
wrote:

Closed #4880 #4880.


Reply to this email directly or view it on GitHub
#4880 (comment).

@oliviertassinari
Copy link

@bryanrsmith What's you work around regarding this point?
It seems that disabling the rule is the best option for Material-UI.

@bryanrsmith
Copy link
Contributor Author

@oliviertassinari For now I used the varsIgnorePattern workaround described above.

@gregberge
Copy link

I have the same problem, is it possible to tweak using a plugin?

@oliviertassinari
Copy link

@neoziro I think that https://github.com/babel/eslint-plugin-babel would be the right location to add the support for this feature.

@bryanrsmith
Copy link
Contributor Author

I've written a (hopefully temporary) plugin for this at https://github.com/bryanrsmith/eslint-plugin-no-unused-vars-rest. It basically just monkey patches the option into the existing core rule.

I'd be happy to submit a PR to eslint-plugin-babel if everyone would prefer that. It's a little odd, though, because it doesn't require babel-eslint. Espree's experimentalObjectRestSpread option covers it. What do you folks think?

@mnpenner
Copy link

mnpenner commented Oct 6, 2016

Thank you @bryanrsmith. That's exactly what I was looking for! Was just about to disable no-unused-vars altogether because of this.

@uniqname
Copy link

Is this not already a valid, current spec compliant pattern when used with array destructuring?

const [head, ...tail] = someArray;

@mmcgahan
Copy link

mmcgahan commented Dec 5, 2016

@uniqname this is for Object destructuring assignment, which is still a Stage 3 proposal, separate from Array destructuring, which is an ES2015 standard.

@uniqname
Copy link

uniqname commented Dec 5, 2016

@mmcgahan I understand that, but the no-unused-vars option does not appear to support this pattern for array destructuring either. I can open a separate issue if that is preferable, unless I am mistaken and this option is supported for this pattern with array destructuring.

@not-an-aardvark
Copy link
Member

With array destructuring, you can use this as a workaround:

const [ , ...tail] = someArray;

This isn't possible with object destructuring.

@uniqname
Copy link

uniqname commented Dec 5, 2016

Interesting. I'm not sure I would prefer that to a named destructured variable that is optionally ignored by islint, but at least the pattern is supported. Thanks @not-an-aardvark

mheiber added a commit to nymag/clay-space-edit that referenced this issue May 15, 2017
doesn't work with destructuring. eslint/eslint#4880

ES2015 is like three years old, @open-source-maintainers.
mheiber added a commit to nymag/clay-space-edit that referenced this issue May 15, 2017
doesn't work with destructuring. eslint/eslint#4880

ES2015 is like three years old, @open-source-maintainers.
@jameswomack
Copy link

For posterity's sake:
https://eslint.org/docs/rules/no-unused-vars#ignorerestsiblings

The ignorerestsiblings option for the no-unused-var rule accounts for this use case now.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion triage An ESLint team member will look at this issue soon
Projects
None yet
Development

No branches or pull requests

10 participants