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

Prevent noisy destructuring on function parameter #97

Merged
merged 5 commits into from Mar 14, 2022

Conversation

Fryuni
Copy link
Member

@Fryuni Fryuni commented Mar 14, 2022

Summary

As requested by @marcospassos, this new rule prevents noisy destructing on function parameters.

It allows restructuring in the function parameter if the function has only one parameter and the entire destruction is in a single line.
Any other destructing in a function parameter is not allowed.

Examples on the docs

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@Fryuni Fryuni added the feature New feature label Mar 14, 2022
@Fryuni Fryuni self-assigned this Mar 14, 2022
Copy link
Member

@marcospassos marcospassos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fryuni, there are some valid cases that I see no problem in allowing:

const foo = ({bar}) => bar;
const foo = (bar, {quz}) => quz;

We should completely disallow multiline restructuring in arguments. Everything else is acceptable to me, provided it doesn't exceed the max line length.

docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
docs/rules/parameter-destructuring.md Outdated Show resolved Hide resolved
src/rules/parameter-destructuring/index.ts Show resolved Hide resolved
@Fryuni
Copy link
Member Author

Fryuni commented Mar 14, 2022

We should completely disallow multiline restructuring in arguments. Everything else is acceptable to me, provided it doesn't exceed the max line length.

@marcospassos I personally see these as a problem:

const foo = ({some, value}, {yet, another, value}) => {};

I feel like the meaning of the parameters is lost, but I'm not entirely against it. I'll remove the single parameter restriction and the option to allow single line since we'll always have it enabled

@marcospassos
Copy link
Member

I see your point, but I prefer to allow it since I can't even remember an example of a function that takes two objects. In general, we always declare options as the last parameter, reason why I'm against only allowing destructuring on the first parameter (i.e. public evaluate(expression: string, {timeout}: Options))

Fryuni and others added 3 commits March 14, 2022 15:05
@marcospassos marcospassos merged commit 28a17bb into master Mar 14, 2022
@marcospassos marcospassos deleted the forbid-multiline-parameter-destructuring branch March 14, 2022 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants