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

Option Request: Multi-line formatting of object destructuring #4384

Closed
tennisgent opened this issue Apr 26, 2018 · 5 comments
Closed

Option Request: Multi-line formatting of object destructuring #4384

tennisgent opened this issue Apr 26, 2018 · 5 comments
Labels
area:destructuring lang:javascript Issues affecting JS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@tennisgent
Copy link

tennisgent commented Apr 26, 2018

A recent change from #4267 resulted in an intentional change in how objects are destructured. I've been using prettier now for a long time and I love it. But I used to be able to do something like this:

(edited by @duailibe)

Before

const { match: { params: { id } } } = this.props  // 1 line

After

const { 
    match: { 
        params: { id }   // 5 lines
    } 
} = this.props

And there are a number of other places where this has caused a number of extra lines of code (that IMO are less readable). So I'm requesting that this change be given a config option so I can disable just this one rule.

Thank you for your consideration, time and for putting such great work into prettier

@duailibe duailibe added status:needs discussion Issues needing discussion and a decision to be made before action can be taken lang:javascript Issues affecting JS labels Apr 26, 2018
@alexander-akait
Copy link
Member

👎 on option
@tennisgent please provide example of code where you have less readable places

@j-f1
Copy link
Member

j-f1 commented Apr 26, 2018

How about only expanding the node onto multiple lines if:

  • there are more than two keys in it that have children that are objects, OR
  • one of its children has an object child that is expanded

@duailibe
Copy link
Member

duailibe commented Apr 26, 2018

Hi @tennisgent, thanks for the report!

First of all, the catch one was unintended. For example, when in function arguments, we require one extra "level" in the hierarchy to break the destructuring, see the example in the playground. So in your specific example, it should behave the same.

I appreciate your comment and I certainly prefer the previous behavior, but we won't be adding any options for that. Instead, I'd like to tweak our heuristics to print them better.

Let's keep in mind this has been discussed at length on #2550

@duailibe
Copy link
Member

duailibe commented Apr 26, 2018

Oh the handleClick case is already printed correctly:

Prettier 1.12.1
Playground link

Input:

function fn({target: {value}}) {}

const fn = ({target: {value}}) => {}

Output:

function fn({ target: { value } }) {}

const fn = ({ target: { value } }) => {};

@duailibe
Copy link
Member

duailibe commented May 1, 2018

@tennisgent I edited the OP to remove the outdated examples. If you have more, report back!

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Oct 9, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Oct 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:destructuring lang:javascript Issues affecting JS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken
Projects
None yet
Development

No branches or pull requests

5 participants