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

Keep multi-line formatting for destructuring assignment #2550

Open
Brian-Gaffney opened this issue Jul 27, 2017 · 134 comments
Open

Keep multi-line formatting for destructuring assignment #2550

Brian-Gaffney opened this issue Jul 27, 2017 · 134 comments
Labels
area:destructuring lang:javascript Issues affecting JS status:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@Brian-Gaffney
Copy link

Brian-Gaffney commented Jul 27, 2017

I think multi-line destructuring should not be collapsed by Prettier.

Same concept as the multi-line objects special case.

When doing destructuring (particularly nested) new lines can greatly enhance readability.

Example of current behaviour
Input:

// Object with multiple lines uncollapsed by Prettier
const user = {
  name: {
    first: "Zoid",
    last: "Berg",
  },
};

// Similar destructuring assignment is collapsed by Prettier
const {
  name: {
    first,
    last,
  },
} = user;

Prettier output:

// Object with multiple lines uncollapsed by Prettier
const user = {
  name: {
    first: "Zoid",
    last: "Berg"
  }
};

// Similar destructuring assignment is collapsed by Prettier
const { name: { first, last } } = user;
@azz
Copy link
Member

azz commented Jul 27, 2017

The output seems perfectly readable to me.

The special-casing of objects is more of a failing in prettier than a feature, I don't think it makes sense to spread that to more cases.

@Brian-Gaffney
Copy link
Author

I agree that the above example is quite readable, I was only going for a minimal example case.

For larger and deeper destructuring assignments I find it harder to read.

Larger example
Input:

const {
  name: {
    first,
    last,
  },
  organisation: {
    address: {
      street: orgStreetAddress,
      postcode: orgPostcode,
    },
  },
} = user;

Prettier output:

const {
  name: { first, last },
  organisation: { address: { street: orgStreetAddress, postcode: orgPostcode } }
} = user;

If there's no interest in this change then I'm fine to drop it.
I found it a bit of a barrier to adoption at work where we have some very big props destructuring in React components and I thought others might have the same issue.

@azz
Copy link
Member

azz commented Jul 27, 2017

Agreed, the input looks better.

Maybe a better rule would be "always break nested object patterns onto multiple lines", rather than looking at the source. Though that rule might have to only apply to variable declarations, else this will be an issue:

function f({ data: { name } }) {}
//Out:
function f({
  data: {
    name
  }
}) {}

Which would likely be seen as a regression.

@Brian-Gaffney
Copy link
Author

For React pure render functions I do find myself writing them similarly to your example output above.
Each destructed variable is on a new line.

export default UserComponent ({
  name: {
    first,
    last,
  },
  organisation: {
    address: {
      street: orgStreetAddress,
      postcode: orgPostcode,
    },
  },
}) {
  return (
    <div>
      ...
    </div>
  )
};

I'd be fine with destructuring always using multiple lines but I imagine many others won't share that opinion.

Are you against referencing the source formatting because it's difficult or because it's a special case and against the general spirit of Prettier?

@azz azz added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Jul 27, 2017
@azz
Copy link
Member

azz commented Jul 27, 2017

Are you against referencing the source formatting because it's difficult or because it's a special case and against the general spirit of Prettier?

The latter. But moreso because it makes prettier less predictable. Having to learn "tricks" to get prettier to print optimal code sucks. An example I run into all the time:

Object.assign({
  foo: bar
}, baz);

Will be printed as

Object.assign(
  {
    foo: bar
  },
  baz
);

When I usually want

Object.assign(
  { foo: bar },
  baz
);

Which you can convince prettier to do by removing the line breaks, but not everyone knows that.

@alexrqs
Copy link

alexrqs commented Aug 13, 2017

@Brian-Gaffney makes an excelent point, I use that destructuring a lot on react pure components and I'm a little tired of using // prettier-ignore plus I found myself on this scenario:

when you add or remove a key the diff will look like

import {
  Foo,
  Bar,
-  Baz, 
}

but with prettier the hole line has changes

- import { Foo, Bar, Baz } from './Qux'
+ import { Foo, Bar } from './Qux'

PS - Going for the minimal example here but keep in consideration real use cases like

  name: {
    first,
    last,
  },
  organisation: {
    address: {
      street: orgStreetAddress,
      postcode: orgPostcode,
    },
  },
}

@SimenB
Copy link
Contributor

SimenB commented Aug 28, 2017

I just opened up #2685 as a dupe of this.

I'll reiterate my stance from that here.

I would expect them to be treated the same - either both collapse, or both stay expanded.
(Personally I would prefer both to be expanded, combined with #2068 being a thing you'd invoke sometimes)

Same with ES6 named imports, FWIW.

@lewisl9029
Copy link

lewisl9029 commented Aug 29, 2017

Maybe we can limit multi-line formatting to only destructuring statements with 2 or more props (possibly also make it configurable)? Similar to the ESLint minProperties option on their multi-line object rules (https://eslint.org/docs/rules/object-curly-newline#minproperties). This would still ignore existing formatting so prettier can format things in a consistent way.

I'd love to have similar (configurable) behavior for objects, arrays and function args too.

@Panya
Copy link

Panya commented Aug 29, 2017

@alexrqs yep, diff-friendly code style is a good thing.

@morgs32
Copy link

morgs32 commented Sep 7, 2017

Not to sound overeager, but these sound like good reasons to me. Next I'd like to add a trailing comma.

What do we need to do to keep the discussion moving forward. Perhaps it can be weaved into #74 ? Is it right to suggest that this format would be consistent with the way we (want to) treat object literals and lists?

#15 (comment)

@nocke
Copy link

nocke commented Oct 3, 2017

If you also have less/css files in mind, consider scenarios like these:

body,div,dl,dt,dd,ul,ol,li,
h1,h2,h3,h4,h5,h6,
pre,code,form,fieldset,legend,input,textarea,p,blockquote,
th,td,hr,button,article,aside,details,figcaption,figure,
footer,header,hgroup,menu,nav,section {
  margin: 0;
  padding: 0;
}

Nobody want every of those selectors on a separate line. Which currently happens. (and using // prettier-ignore in too many places is no fun, either). Is there a way to generally suppress this for .less-files?

A linefeed right before printWidth gets reached (before more comma-separated selectors on the next line) would be good... or an option to completely leave them alone (as you can see above, those linefeeds kinda structure them, i.e. all headline-sectors together...)

@lydell
Copy link
Member

lydell commented Oct 3, 2017

@nocke Your example is not very good for suggesting different formatting. You only do that reset once, right? So it doesn't matter that much if that one rule gets a 20-line selector (in my opinion).

@sophistifunk
Copy link

Is the point of prettier to allow teams to agree on and enforce a style mechanically? Or is it to export the authors' particular preferences to the world at large?

If the former, then I think it would benefit one of:

  • Respecting the original intent as the object syntax formatting does
  • Using a max-item-count for both objects and destructure statements
  • Allowing a character-count limit for single-line objects and destructure statement separate from line width

This is a big sticking point with some members of my team - I'm personally willing to pay the price of a few ugly destructures in order to get mechanical formatting, but I don't write the cheques.

@lydell
Copy link
Member

lydell commented Oct 11, 2017

is it to export the authors' particular preferences to the world at large?

Definitely not that one. Sometimes issues need a lot of discussions before the best solution to go forward is found. Like the JSX ternary change. And sometime everyone ends up working on bugs or other features instead.

@suchipi
Copy link
Member

suchipi commented Oct 12, 2017

Maybe a better rule would be "always break nested object patterns onto multiple lines", rather than looking at the source. Though that rule might have to only apply to variable declarations - @azz

I'm in favor of this- that said, I never use nested destructuring, partially because I prefer to keep prop shapes pretty shallow, and partially because I find it difficult to grok.

@nocke
Copy link

nocke commented Oct 12, 2017

2 real-world javascript examples:

A) There are times, where you just want things as a big blob (my first example) –not wasting vertical space to scroll over– that's hardly ever to change:

const npmCommands = [
    'access', 'add-user', 'adduser', 'apihelp', 'author', 'bin', 'bugs', 'c',
    'cache', 'completion', 'config', 'ddp', 'dedupe', 'deprecate', 'dist-tag',
    'dist-tags', 'docs', 'edit', 'explore', 'faq', 'find', 'find-dupes', 'get',
    'unlink', 'unpublish', 'unstar', 'up', 'update', 'upgrade', 'v', 'version',
    'view', 'whoami' ];

B) ...in the very same project there are places, where you want everything on it's own line for sake of maintainability, even if it amounts to dozens of lines:

const slugify: HashMap = {
	'£': 'pound',
	'¥': 'yen',
	'ß': 'ss',
	'à': 'a',
	'á': 'a',
	'â': 'a',
	'ã': 'a',
	'ä': 'ae',
	...

Meaning:
An auto-mode that detects (no rocket since, rather simple: are there „mostly“ linebreaks between the object/array members/css selectors or not?) and then reinforces that on any minor inconsistencies... (as prettier should do).

@smallnamespace
Copy link

smallnamespace commented Nov 30, 2017

As a side note, ESLint has options to allow breaking these nested objects into multiple lines vs. keeping them consolidated.

At the minimum, it seems that prettier should be configurable to accept consolidated or broken-apart nested objects since the best style seem tricky to define except on a case-by-case basis.

@nocke
Copy link

nocke commented Dec 1, 2017

At the minimum, it seems that prettier should be configurable to accept consolidated or broken-apart
nested objects since the best style seem tricky to define except on a case-by-case basis.

I would see (an option for) auto-detection as a good case-by-case basis. (see above) Do not rely on length or any other assumption, rely an what's there (and then consolidate, if the apparently chosen style is only „mostly“ there...).

By the way, js-beautify has something like that, appending a ,preserve-inline

"brace_style": "collapse,preserve-inline",

tested, and it works pretty nicely...

@machineghost
Copy link

machineghost commented Jan 21, 2018

Regarding:

Meaning:
An auto-mode that detects (no rocket since, rather simple: are there „mostly“ linebreaks between the object/array members/css selectors or not?) and then reinforces that on any minor inconsistencies... (as prettier should do).

and:

I would see (an option for) auto-detection as a good case-by-case basis. (see above) Do not rely on length or any other assumption, rely an what's there (and then consolidate, if the apparently chosen style is only „mostly“ there...).

I'm not in any way affiliated with the project, but based on my understanding of it that would be impossible. Specifically I'm referring to the "detects ... are there mostly linebreaks between ..." part and the "rely on what's there" part. As I understand it prettier reduces your code to an AST first; that's part of the "magic" of how it can format files consistently. And doing that amounts to discarding all formatting that was there previously.

In other words, Prettier can't do anything based on the way you had formatted something, because it is designed to throw out everything you had before and just generate a consistent result.

I think the real fix here would be to give users more control over how Prettier output things every time. That is a request I'd definitely 👍

@j-f1
Copy link
Member

j-f1 commented Jan 21, 2018

In other words, Prettier can't do anything based on the way you had formatted something, because it is designed to throw out everything you had before and just generate a consistent result.

That’s not entirely true — Prettier does have ways of accessing the original source, and that’s used to keep multline objects multiline and preserve (single) blank lines. There is a feature request somewhere for an option to disable this and only use the AST, though.

@josephfrazier
Copy link
Collaborator

There is a feature request somewhere for an option to disable this and only use the AST, though.

Here it is, for reference: #2068

@narkowicz
Copy link

As a new user testing Prettier in a fresh project, the newline behaviour around all destructuring (including import statements) and arrays, along with the closely related issues regarding jsx #3101 and json #2716, are the only deal-breakers preventing adoption.

Our own reasons for using multi-line formatting in imports, destructures, objects, arrays, etc. relate mainly to supporting simple / automatic re-ordering and easy addition and removal of props, all of which help maintain clean git history (i.e. when used with trailingComma).

As mentioned in #3101 (comment) code reviews benefit from multi-line props - this extends to merge operations - and the same applies when destructuring.

Without this configuration option, it's possible we would see more, not less, formatting related changes in our git history which is at odds with our own motivations for implementing Prettier.

FWIW the only time I wouldn't prefer a multi-line destructure is in an inline arrow function such as:

[
  {
    key: 'value',
  },
].map(({ key }) => key);

...but given the choice I would rather have the option to always enforce newlines when destructuring than support edge cases like this.

The running suggestion in #3101 (comment) of defining the max number of props per line seems like it could carry across for all destructuring and import statements as well. i.e. a value of 0 would always enforce multi-line, 1 allows for a single inline destructure (as in example above), and 3 inline props seems to be the magic number for those not attached to blanket enforcement. Could this be combined with another option to respect intent as per #2550 (comment) ?


As an aside, the current destructure formatting seems inconsistent with the formatting applied when constructing an object- which appears to respect the author's intent. i.e. the following passes through formatting as-is:

const a = 0;
const b = 1;
const c = { a, b };
const d = {
  a,
  b,
};

(although this is not the case with arrays...)

@azz
Copy link
Member

azz commented Jan 22, 2018

Could we just always break onto multiple lines if there's nested destructuring, and keep it as it is for flat destructuring?

@narkowicz
Copy link

If it warrants a configuration option I'd love to see a setting that supports always breaking.

@azz to illustrate my preference, compare the following diffs:

# Breaking nested destructure only
-const { a, b, c } = someObject;
+const {
+  a,
+  b,
+  c,
+  d: {
+    e,
+  },
+} = someObject;
# Always breaking (clear addition, minimal delta)
const {
   a,
   b,
   c,
+  d: {
+    e,
+  },
 } = someObject;

Meanwhile changes in another branch now need to be merged with the nested destructure formatting - each case results in a conflict - compare the two diffs:

# Breaking nested destructure only (conflict must be resolved manually)
-const {
-  a,
-  b,
-  c,
-  d: {
-    e,
-  },
-} = someObject;
+const { a, b, c, f, g } = someObject;
# Always breaking (easily resolved with merge tool / editor by accepting both changes)
const {
   a,
   b,
   c,
-  d: {
-    e,
-  },
+  f,
+  g,
 } = someObject;

@azz
Copy link
Member

azz commented Jan 22, 2018

We can't always optimize for diffs. Imagine if we printed every function like this:

function foo(
  a,
  b
) {}

We don't want to do that, and your example is similar. e.g.:

const {
  a,
} = b;

// or

const f = ({
  a,
}) => a;

@mlshv
Copy link

mlshv commented Aug 2, 2021

Hello people! Is there any work happening on this feature? Is prettier going to make my code pretty? 🙂

@G1itcher
Copy link

I hate to do this but I'd really like to see an update on this one.

@ldaza-w
Copy link

ldaza-w commented Feb 8, 2022

image

look at this... this just looks bad.. #uglier

@danarchos
Copy link

danarchos commented May 31, 2022

Do you really think this is prettier

const { text, hitSlop, onPress, style, activeOpacity, testID } =
  this.props;

than this..

const {
  text,
  hitSlop,
  onPress,
  style,
  activeOpacity,
  testID 
} = this.props;

??

@pvinis
Copy link

pvinis commented May 31, 2022

Do you really

that's why prettier is a bad tool. because some people prefer the first, and some the second.

@jimfilippou
Copy link

Do you really think this is prettier

const { text, hitSlop, onPress, style, activeOpacity, testID } =
  this.props;

than this..

const {
  text,
  hitSlop,
  onPress,
  style,
  activeOpacity,
  testID 
} = this.props;

??

I do, I also respect @pvinis's opinion since the concept of "pretty" differs from person to person.
Using an opinionated tool against your own opinion can be a determining factor in choosing prettier or not.

@danarchos
Copy link

@jimfilippou I understand what you are saying, but howcome there are other configurations for the user to change settings based on what they think is "pretty", whereas with this there is not.

Some people prefer single quotes, others double, that's easily switched. Prettier gives you this option as it does not assert which one it thinks is prettier.

@amcinerney
Copy link

Keeping this alive. Yet another person who would like to see an option for this. I much prefer deconstruction/object fields to be on new lines for readability

@douglasg14b
Copy link

I to want an option for this, being able to say Don't condense multi-line objects to a single line would be nice to have, because there are some conventions where we WANT multi-line object,s but prettier insists that we cannot, with no clear way to disable or configure that behavior.

@gaievskyi
Copy link

@Brian-Gaffney makes an excelent point, I use that destructuring a lot on react pure components and I'm a little tired of using // prettier-ignore plus I found myself on this scenario:

when you add or remove a key the diff will look like

import {
  Foo,
  Bar,
-  Baz, 
}

but with prettier the hole line has changes

- import { Foo, Bar, Baz } from './Qux'
+ import { Foo, Bar } from './Qux'

PS - Going for the minimal example here but keep in consideration real use cases like

  name: {
    first,
    last,
  },
  organisation: {
    address: {
      street: orgStreetAddress,
      postcode: orgPostcode,
    },
  },
}

That's actually perfect. I also still want to see an option for this.

@ZaLiTHkA
Copy link

ZaLiTHkA commented Aug 8, 2022

at the risk of throwing a spanner in the works, I just had completely random thought: could Prettier perhaps format these sections conditionally based on the existence of explicit trailing commas?


drawing a little inspiration from the Dart formatter here: if code is written with a final trailing comma, then the formatter puts each entry on it's own line. whereas without this final trailing comma, it puts each entry on the same line (where applicable).

naturally, to not change current default behaviour (i.e.: usage with no explicit configuration), a change like this would need to be hidden behind an appropriate configuration flag though..

but I imagine a boolean-style "respect-trailing-comma" toggle could work quite nicely, where setting this to true would then format based on the existence of trailing commas that (this tool should assume) have been explicitly inserted by the dev.


FWIW, the following Dart snippet was all formatted in a single file:

var listExampleA = [
  'word',
  'more words',
  'final word',
];

var listExampleB = ['word', 'more words', 'final word'];

void funcExampleA(var arg1, var arg2) {}

void funcExampleB(
  var arg1,
  var arg2,
) {}

@adrian-gierakowski
Copy link

For anyone looking for an alternative formatter, check out https://github.com/dprint/dprint-plugin-typescript

it’s blazing fast and is reasonably configurable so that the choice of multi-line vs single line can be left to the developer

@waynebloss
Copy link

waynebloss commented Aug 22, 2022

Here's another alternative that implements this feature: deno fmt, the Deno code formatter

This can be used in VS Code via the Deno extension. Just set your default formatter to it and you can leave the deno.enable VS Code setting false for your non-Deno projects.

Of course, if you are doing formatting in your CI/CD pipeline you just have to install deno there too.

{
"[typescript]": {
    "editor.defaultFormatter": "denoland.vscode-deno"
  },
}

@ZaLiTHkA
Copy link

I discovered a Prettier plugin earlier today through this comment, which covers my needs with regards to JS arrays, but not for objects, as required for this issue..

since I've never tried my hand at creating Prettier plugins before, I might be missing something.. but perhaps prettier-plugin-multiline-arrays could be extended or forked to cover this scenario as well?

@igorrocha
Copy link

I saw some people had issues in the past with multi-line arrays being put on a singleline by Prettier, and their workaround was to add a comment after the array's first element. I've tested with destructuring and it works too, even though it's not a great solution.

Writing your destructuring like this will keep it untouched by Prettier

const {
    prop1, //
    prop2,
    prop3,
} = props

@OJFord
Copy link

OJFord commented Feb 7, 2023

I do a similar thing for imports:

import {
  foo,
  //
} from "bar"

but it's annoying to need to.

It's not that this is necessarily 'prettier' than { foo } itself, it's that the resulting diffs are a hell of a lot prettier/more readable/more usable.

The special-casing of objects is more of a failing in prettier than a feature

The failing is that it doesn't insist on readable diffs, and allows you to single-line it all if you remove the newline.

@teksrc
Copy link

teksrc commented Mar 3, 2023

Is it just me, or is it simply ridiculous that after 6 years this issue is still open and still there isn't multiline option for object deconstruction in prettier formatting with specificity for how many values are deconstructed before it breaks into multi line?

@adrian-gierakowski
Copy link

@Frankcarvajal

prettier is dead

use dprint: #2550 (comment)

@bodenloszt
Copy link

bodenloszt commented Mar 7, 2023

Is it just me, or is it simply ridiculous that after 6 years this issue is still open and still there isn't multiline option

honestly :)). i was reading this entire issue and hoped to find the answer i need but well, surprise

@cpatti97100
Copy link

HI all, come on I think we can do this 🙏🏼 thanks again for this great tool :)

@sshmaxime
Copy link

Hi all, well, my turn now to wake things up 🙏

@superchangme
Copy link

Hi all, well, my turn now to wake things up 🙏

has any new changes about this?
i use eslint rule

 // 建议6 对象字面量属性超过2个, 需要都换行
    'object-curly-newline': [
      'error',
      {
        ObjectExpression: {
          minProperties: 2,
          multiline: true,
          consistent: true
        },
        ObjectPattern: {
          minProperties: 6,
          multiline: true,
          consistent: true
        }
      }

but prettiter format me to one line,my brain want boom

@resistdesign
Copy link

I have this, and I hate it! I have to set printWidth down to like 50 to prevent it. 😨
image

@CarlSalaets
Copy link

at the risk of throwing a spanner in the works, I just had completely random thought: could Prettier perhaps format these sections conditionally based on the existence of explicit trailing commas?

drawing a little inspiration from the Dart formatter here: if code is written with a final trailing comma, then the formatter puts each entry on it's own line. whereas without this final trailing comma, it puts each entry on the same line (where applicable).

naturally, to not change current default behaviour (i.e.: usage with no explicit configuration), a change like this would need to be hidden behind an appropriate configuration flag though..

but I imagine a boolean-style "respect-trailing-comma" toggle could work quite nicely, where setting this to true would then format based on the existence of trailing commas that (this tool should assume) have been explicitly inserted by the dev.

FWIW, the following Dart snippet was all formatted in a single file:

var listExampleA = [
  'word',
  'more words',
  'final word',
];

var listExampleB = ['word', 'more words', 'final word'];

void funcExampleA(var arg1, var arg2) {}

void funcExampleB(
  var arg1,
  var arg2,
) {}

This is a very underrated comment.

This gives predictable behaviour, and the flexibility to decide case by case.
It is a lot cleaner than the "comment hack" and it ties in perfectly with what a lot of people are doing anyway (i.e., using trailing comma's in multiline arrays/objects).

@ptrxyz
Copy link

ptrxyz commented May 15, 2024

This issue is open since 2017 and we keep on discussing it. I would highly advocate for implementing the trailing-comma-solution mentioned above -- or any other one, really -- so that we move on for now.
Imho this doesn't need to be perfect, but at least it's better than nothing and we can improve it later on.

@DestinyZxw
Copy link

successHandle({
	item,
	storeItem,
	res,
	resolve
}) {
	const {
	    statusCode,
	    data
	} = res;
	let bodyData;

	const {
	    name,
	    serverUrl
	} = storeItem

	try {
		bodyData = JSON.parse(data);
	} catch (e) {
		this.errHandle({
			item,
			storeItem,
			resolve,
		});
	}
	// xxx more
}	

really cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:destructuring lang:javascript Issues affecting JS 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