Prettier: Angular Output/Input decorators are moved to new line

Created on 1 Aug 2018  Â·  110Comments  Â·  Source: prettier/prettier

Prettier 1.14.0
Playground link

--parser typescript

Input:

@Component({
  selector: 'toh-hero-button',
  template: `<button>{{label}}</button>`
})
export class HeroButtonComponent {
  @Output() change = new EventEmitter<any>();
  @Input() label: string;
}

Output:

@Component({
  selector: "toh-hero-button",
  template: `<button>{{label}}</button>`
})
export class HeroButtonComponent {
  @Output()
  change = new EventEmitter<any>();
  @Input()
  label: string;
}

Expected behavior:
Decorators are kept on the same line.

More details:
When using Angular, it's recommended to follow their style guide, which specifies:

Placing the decorator on the same line usually makes for shorter code and still easily identifies the property as an input or output. Put it on the line above when doing so is clearly more readable.

Angular classes are a valid (and common) use-case for Prettier, but with Prettier 1.14 (relevant PR) it deviates from the Angular style guide, causing unwanted changed for most people using Angular.
Note that the above code example (input) is taken directly from the Angular style guide.

The original PR to change this behavior mentions that mobx is the only library using same-line decorators, which is clearly not the case, with Angular being a (big) second one.

To avoid reverting the new behavior completely, which is probably still wanted by some people - I suggest adding an option to the config to keep the old behavior, or prefer the new one (default should be discussed further).

help wanted javascript locked-due-to-inactivity bug

Most helpful comment

In Angular, property decorators look and feel like modifiers. Like private, static, readonly, etc., only custom. Having @Input() on a separate line is as unusual as having private on a separate line.

All 110 comments

Please no, we just fought so hard for this. Decorators on the same line are absolutely horrible to read and I read a lot of code. I need to be able to quickly find variables and their assignments and a mix ruins the flow. Why angular ever recommended inline decorators is beyond me. And while it is a recommendation with both sets of language it does not deviate. The last line “put it on the line above when doing so is clearly more readable” is, in a lot of our opinions, always true. I absolutely can’t stand inline decorators about as much as I can’t stand constructor accessibility modifiers.

Please no, don't add more unnecessary options.

Decorators/attributes are always in their own line in all other languages + the decorator spec itself. No need to start a new trend here.

https://github.com/prettier/prettier/issues/2613#issuecomment-401940927

Angular and MobX (btw mobx is a library I love, but regardless) maintainers should not come up with incompatible style decisions.

In Angular, property decorators look and feel like modifiers. Like private, static, readonly, etc., only custom. Having @Input() on a separate line is as unusual as having private on a separate line.

@johnpapa @IgorMinar @github/angular What's your opinion?

@thorn0 why don't they use the same functionality from typescript? IMHO, it doesn't make sense to inline them just because there is a single specific (and, again IMHO, hacky[1]) use-case which imitates some usually-inline syntax.

[1] I fully respect the developers who came with this. They most probably had their reasons. This doesn't however affect my point.

We're not going to add an across-the-board option for this; see prior discussion in #2613 and #325.

We might be open to a solution that meets Angular users' needs as long as it doesn't cause inlining for everyone else. But I don't think detecting @Input() and @Output() is safe, because those could be used in other libraries- we've recently gotten a little flak for detecting pipe(...) as a function programming composer function when it's often something else.

Also, I feel we are starting to go down a slippery slope of special-casing things for certain libraries. It's not maintainable long-term.

@suchipi I'm not suggesting detecting anything automatically, but rather have an option in the config file to either give decorators their own line, or keep them inlined (for class properties at least).

This is not ideal, since it further expands the config options that prettier has, which should be minimal by definition, with it being opinionated. However, I think not having it (or another solution) removes a good portion of potential users from using Prettier, for a rather small issue that could be solved.

@bengry: From @suchipi’s comment:

We're not going to add an across-the-board option for this; see prior discussion in #2613 and #325.

@egeozcan You misread my comment. I didn't say they have the same semantics as private, etc. However, just like private, property decorators are little markers that don't deserve separate lines.

If a property decorator's length (with its arguments) is long enough (>15 characters, probably), then of course they don't look like little modifiers any more and need a separate line.

How about if inlining the decorator is named either /[A-Z][a-z0-9]+/ or /[a-z0-9]+/ (i.e. Foo or foo but not FooBar)?

That seems like it would surprise users...

@thorn0

Would you like something like this to be common in JS:

class Test {
    @IsNotToBeInlined
    label: string;
    @IsToBeInlined otherLabel: string;
}

...while literally every language ever invented a) has all its style guides having them on their own line or b) doesn't even allow them inline?

private, protected and other access modifiers are not user definable and it is common to group properties with common modifiers together to have the names aligned.

On the other hand, attributes can be as long (or short) as the developers want. This is the main reason why they get their own line. Sorting them by length to improve the roller-coaster reading experience may not make logical sense. If you start inlining them, it is proved to be impossible (by many style-guide authors, as evidenced by their mail lists and the guides themselves) to find a logical point where it's ok for them to be inlined.

If you are only using them with Angular, sure. This may seem like a lot of bike-shedding. Yet, try using them in bigger projects with many decorators coming from various sources. In this case, should Angular coding style be changed when it's used with, say, TypeORM?

TL;DR: We should learn from other communities, and not re-invent things.

^agreed and even the missing line space between decorator and previous declaration is irking me. Please keep decorators on separate line to have one consistent pattern for decorators.

it is common to group properties with common modifiers together to have the names aligned.

And guess what, people do exactly same thing with decorators.

In addition: In Python, they tried to allow more decorators per line (not even the same line as the property) in 2.4a2. They reverted that on a3 and finalized on 2.4 stable that every one of them gets a new line because otherwise it was too inconsistent. I remember the discussion in the mail lists.

Please just look at all the decorator/attribute/whatever-it-is-called examples from all the languages. It is like that for a reason.

And guess what, people do exactly same thing with decorators

Respectfully, I suggest you to read the whole argument around the part you quoted.

Working with Angular, I totally like the behavior of Prettier 1.13: 1) if a property declaration is too long to fit on one line, the decorator is the first thing that is moved to a separate line; 2) if there is more than 1 decorator, they get separate lines.

Quoting myself here, maybe you have missed it:

If you are only using them with Angular, sure. This may seem like a lot of bike-shedding. Yet, try using them in bigger projects with many decorators coming from various sources. In this case, should Angular coding style be changed when it's used with, say, TypeORM?

@egeozcan I don't think there's much more about opinion to be added to the discussion here. Don't worry, we won't revert https://github.com/prettier/prettier/pull/4830.

Looking at example code for TypeORM, I noticed something interesting:

@Entity()
export class PhotoMetadata {
    @PrimaryGeneratedColumn()
    id: number;

    @Column("int")
    height: number;

    @Column("int")
    width: number;

    @Column()
    orientation: string;

    @Column()
    compressed: boolean;

    /// ...
}

They had to add blank lines between properties, otherwise it becomes barely readable:

@Entity()
export class PhotoMetadata {
    @PrimaryGeneratedColumn()
    id: number;
    @Column("int")
    height: number;
    @Column("int")
    width: number;
    @Column()
    orientation: string;
    @Column()
    compressed: boolean;
    /// ...
}

Which basically means that with Prettier 1.14, every property that used to need only one line before, now needs 3.

Prettier should add those blank lines too then. Without them, it's a mess.

^absolutely. But my understanding was that prettier honored blank lines that users add. So if you have them bunched up they will rendered bunched but do you add the space it will be honored. Is that not the case?

It is the case of course. But imagine Prettier 1.14 reformatting a big Angular or MobX project with decorators inlined and no blank lines between properties. The output won't be pretty.

@egeozcan For the record, what you wrote about "literally every language ever invented" is a pretty bold statement. At least, in the C# world it's not a universal truth. People use attributes in many different ways. E.g. see https://github.com/dotnet/roslyn/issues/11572

@thorn0 being alowed by the compiler does not mean it's in any style guide. ReSharper and VS automatically move them on their own lines. I said (please note the "or"):

...while literally every language ever invented a) has all its style guides having them on their own line or b) doesn't even allow them inline?

@suchipi sorry for the noise :) and thanks for the clarification. I'm out.

ReSharper and VS automatically move them on their own lines

It's simply not true. VS 2017 doesn't do so.

...while literally every language ever invented a) has all its style guides having them on their own line or b) doesn't even allow them inline?

a) C# style guides say nothing about it, b) they're allowed inline

Google Java style guide on annotations (the use of bold is mine):

Annotations applying to a field also appear immediately after the documentation block, but in this case, _multiple_ annotations (possibly parameterized) may be listed on the same line; for example:

@Partial @Mock DataLoader loader;

Chiming in here, we also have similar issues with decorators on fields in Ember.js. For certain types of fields that are declared using decorators, it ends up being much more readable to have everything be on the same line, especially with Prettier's rules about whitespace.

export class DropdownComponent extends Component {
  @argument placement;
  @argument position;
  @argument trigger;

  didInsertElement() {
    // ...
  }

  @action
  onOpen() { 
    // ...
  }
}

export class User extends Model {
  @attr firstName;
  @attr lastName;
  @attr email;

  @belongsTo team;

  @hasMany posts;
  @hasMany comments;
}

// vs

export class DropdownComponent extends Component {
  @argument 
  placement;

  @argument 
  position;

  @argument 
  trigger;

  didInsertElement() {
    // ...
  }

  @action
  onOpen() { 
    // ...
  }
}

export class User extends Model {
  @attr 
  firstName;

  @attr 
  lastName;

  @attr 
  email;

  @belongsTo 
  team;

  @hasMany 
  posts;

  @hasMany 
  comments;
}

As you can see, we can no longer really have "blocks" of declarations with spacing in between. We could have fields one after another with no empty lines, but it makes it even harder to read IMO.

For us, I think it would be ideal to inline _single_ decorator usage _without_ arguments. More than one decorator results in multiline, any arguments results in multiline.

As you can see, we can no longer really have "blocks" of declarations with spacing in between. We could have fields one after another with no empty lines, but it makes it even harder to read IMO.

Agreed.

For us, I think it would be ideal to inline single decorator usage without arguments. More than one decorator results in multiline, any arguments results in multiline.

This seems like a reasonable compromise to me...

Personally, as an Angular developer, I'd much prefer that decorators are inline. I think it looks a lot prettier and saves a lot of vertical space. It also allows grouping which is really nice and is a shame to lose.

But even if we don't put decorators inline (as some people like them such as Angular, MobX and Ember developers, where as TypeORM developers do not), can we at least seperate them by blank lines?* There's no way that this is prettier.

image

*Prettier changes whitespace all the time, that's basically all it does :)

@aboyton as an aside, if you add the blank lines yourself, prettier will respect them.

I don't think inlining when the decorator is either a single identifier or a function call with no arguments is good enough, because that would inline stuff in TypeORM (eg @Column()), which is how we got here...

As you can see, we can no longer really have "blocks" of declarations with spacing in between. We could have fields one after another with no empty lines, but it makes it even harder to read IMO.

agree

For us, I think it would be ideal to inline single decorator usage without arguments. More than one decorator results in multiline, any arguments results in multiline.

agree

Why don't add some configuration option to .prettierrc?
So who's working with TypeORM and who like line break after every decorator use it.

    // yesterday...
    @observable datasourceType = null;
    @observable loading = false;
    @observable loadingViewport = false;
    @observable loadingRows = false;

    // today...
    @observable
    datasourceType = null;
    @observable
    loading = false;
    @observable
    loadingViewport = false;
    @observable
    loadingRows = false;

    // dear god...
    @observable datasourceType = null; // prettier-ignore
    @observable loading = false; // prettier-ignore
    @observable loadingViewport = false; // prettier-ignore
    @observable loadingRows = false; // prettier-ignore

Just go add manual spaces. Those are public properties and are begging for some comments anyway.

I don't think inlining when the decorator is either a single identifier or a function call with no arguments is good enough, because that would inline stuff in TypeORM (eg @Column()), which is how we got here...

An option that would work for us (though unfortunately not Angular devs) would be to only inline decorators and not decorator function calls (e.g. only inline decorators without parens).

Please revert this change ! Angular, Ember and some other frameworks are negatively affected by this. As other have explained the code becomes an unreadable roller coaster.

Horrible, horrible, horrible change, it goes against many well established conventions, there should at least be an option to globally disable this. The changelog says that reports from the community led us to discover that MobX is the only major library that follows this convention. So your solution is to basically say fuck MobX ?

So should I telling my Angular team to change their coding style?
At least make it as an option for teams to choose, please.

I know that you want to limit the config options as much as possible, but this change is highly controversial and I think it would make sense to add an option here. Users of popular frameworks are negative affected, the code is really harder to read.
As a side note: the last comment before the change states that is typically for other languages and style guides that decorators / attributes are on separate rows, which can simply not said in general. For even the example that was posted states clearly that single decorators / annotations may be kept inline:
https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations

sigh

It looks like, in many cases, this change will really hurt readability and make things quite ugly.

It should be noted, that the project name is Prettier, not Small-Configierr.

Maybe just save original input?
Like we do with objects:

obj = { foo: 'bar' };
obj = {
  foo: 'bar'
}; 

Here

@observable datasourceType = null;

@observable
datasourceType = null;

Pros:

  1. No option.
  2. You decide how it looks.
  3. No disputes.

And if someone wants to be sure that all in one style and other developers stick to one style you can create eslint rule for this (maybe it already exists).

Also, there’s been enough support (and opposition) that to these changes that it makes sense not to require a specific style.

It seems like TypeORM users have a louder voice on prettier than angular users. Removing inline decorators means locking prettier on v13 for (a lot of) angular users.

There are 3 ways to go here:

  1. Continue refining the heuristics
  2. Add an option (always same line vs. always separate lines)
  3. Respect original input

The hard part is choosing what to do! _Something_ will be done, we're just not sure what yet.

1 (heuristics)

Pros: The ideal solution.
Cons: Seem almost impossible to come up with the rules?

2 (option)

Pros: Allows enforcing a single style.
Cons: New option. Can't use both TypeORM and Angular with the same config.

3 (preserve)

Pros: Works in all cases.
Cons: Can lead to inconsistent formatting. "Please move this decorator" talk within teams.

Which is less worse than the others?

Both (1) and (3) will work in all cases, while option (2) has some limitations, and involves adding an option. Since option (1) is extremely difficult, I’m in favor of (3). Additionally, if someone were to create a heuristic that correctly determines when to use each style, we would be able to use that heuristic instead.

I think we should do (3), it seems impossible to come up with a satisfactory heuristic

2 (option)
Cons: Can't use both TypeORM and Angular with the same config

I'm sure that most of the time Angular developers who use TypeORM place its decorators inline, the same way they format the rest of their code. Simply to keep the formatting consistent. There is nothing inherently different between TypeORM's and Angular's decorators. The only difference is that the creators of the two projects adopted different formatting conventions. It's like as if TypeORM used only tabs and Angular spaces. Would we have to mix tabs and spaces if we decided to use both libraries in the same project?

@thorn0 Thinking about the config option again, if we always do one thing what would happen with libraries/app-defined decorators that accept "options"-object as their input. e.g.:

export class MyClass {
  @someDecorator({
    name: '...',
    someMoreOptions: {
      idKey: 'id',
    },
    foo: () => 'bar'
  })
  someProperty: string;
}

Are these supposed to be "squashed" and inlined too? This is an obvious case where the decorator should be on its own line.

Also, consider that the someDecorator has most of its options optional, so you could do this as well:

export class MyClass {
  @someDecorator({ name: '...' })
  someProperty: string;
}

Which might make sense to format as:

export class MyClass {
  @someDecorator({ name: '...' }) someProperty: string;
}

I'm starting to think more and more that keeping the original formatting is the best option at the moment, at least until a better heuristic comes along.

@bengry Multi-line decorators would always be on separate lines. Not a problem :)

@bengry

the option can be made obsolete and removed in the next major version of Prettier as a breaking change.

We’ve been planning a major version of Prettier for a long time. It’s likely we’d have to support such an option for a very long time.

@bengry

This is an obvious case where the decorator should be on its own line.

You just answered your own question.

As far as I can tell, the Angular style is to inline only single decorators that look like @Foo, @Foo(), or @Foo('short string').

EDIT (not really relevant but interesting): turns out the Angular team found an interesting solution to the object literal formatting problem:

Note if you explicitly want to force an object literal onto multiple lines (e.g. because you think you'll keep adding to it), you can easily do that by having a trailing comma in it:

I’m an angular developer and can’t stand inline decorators, so we can’t lump all angular developers into the inline decorators camp

@babeal My point was that people don't mix the two styles in one project. It really seems like we have a 'tabs vs spaces' kind of situation here.

Temporary solution for mobx+vscode guys - downgrade prettier-vscode extension:

  1. remove current extension
  2. set "extensions.autoUpdate" to false in settings
  3. download previous version from https://esbenp.gallery.vsassets.io/_apis/public/gallery/publisher/esbenp/extension/prettier-vscode/1.5.0/assetbyname/Microsoft.VisualStudio.Services.VSIXPackage
  4. rename file extension to .vsix
  5. in vscode press CMD+SHIFT+P -> type "vsix" -> Enter
  6. select file and install it
  7. reload vscode

@4urbanoff

Just need to run "npm i -D [email protected]" and wait until this issue is resolved.

https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode
Prettier resolution
This extension will use prettier from your project's local dependencies. Should prettier not be installed locally with your project's dependencies, a copy will be bundled with the extension.

@Avin-Shum
Thank you! Everything was much easier ))

I think better solution i written above (https://github.com/prettier/prettier/issues/4924#issuecomment-412280753)

/cc @vjeux what do you think?

@evilebottnawi no, we shouldn’t solve all the hard problems by respecting how the user entered the code in the first place. We should instead come up with one way of printing code that’s reasonable and enforce it everywhere.

As a mobx developer, I can only lock the version unless this problem is solved.

If an option is still a possibility, I'd like three choices:

  1. Always inline a single decorator (if more than one decorator or line exceeds print width, put decorator(s) on separate line).
  2. Always put decorator(s) on separate line, and surround decorator + property with line breaks.
  3. Leave decorators as is.

@vjeux i don't think we can solve this here. Or respect original input or add option. There is no good way to print this in all cases.

@lydell, Thumbs down because you don't want to add another option (which I get) or you don't like the choices I laid out (which I'd like to hear a little more about)?

As a mobx user, I think I could learn to live with @observable on a separate line, but I would only like this if there's an extra line break after the property declaration.

As in:

@observable
public name = "";

@observable
public location = "";

@observable
public nation = "";

Even though this takes yet more space, it just turns into mush if you don't have it (as @aboyton pointed out).

And @suchipi, yes, I'm aware that prettier will respect line breaks if I add them, but this breaks down when I do a mass formatting of a large project. I'd have to painstakingly go through and insert line breaks. And every time I add a decorator to a property, it won't naturally format correctly. This seems counter to how prettier should work. It should format things to be readable by default.

@devuxer Why do you want three choices for the option?

@lydell,

I think they all support valid use cases:

  1. I'm using mobx or angular (for example), and I don't want a group of observable properties to explode into a whole bunch of lines, forcing me to scroll around a lot to see what's in a class.
  2. I'm using TypeORM, and it's much more readable and conventional to put the decorators on a separate line, so scrolling is worth it.
  3. I'm using a mixture (perhaps both mobx and TypeORM), and I want to be able to mix the two types of formatting.

I definitely don't love the idea of adding another option for this, but I don't see how you satisfy these really common use cases without one. If an option is completely off the table, I would come down on the side of respecting the original position of the decorator (inline vs. separate line). With the previous version of Prettier, I sometimes inlined and sometimes separate lined, and I don't remember ever thinking, "I wish Prettier would help me format my decorators."

So you will set the option to different values for different projects?

Perhaps the common denominator is that, when decorators are grouped, it makes sense to inline, when they aren't it doesn't. So, if you have say 3 or more properties in a row that begin with @observable, inline. If you have a variety of different decorators in a group, don't inline. I'm sure this isn't the perfect heuristic and could be confusing, but perhaps worth thinking about.

Makes sense to inline:

@observable public foo;
@observable public bar;
@observable public baz;

Doesn't make sense to inline:

@observable public foo;
private bar;
@observable private baz;

Input:

private _hidden;

@observable
public foo;
@observable
public bar;

@observable
public baz;

@AllowNull(false) public readonly fizz;
@Column(DataType.FLOAT) public readonly buzz;

Output:

private _hidden;

@observable public foo;
@observable public bar;

@observable public baz;

@AllowNull(false)
public readonly fizz;

@Column(DataType.FLOAT)
public readonly buzz;

@lydell, yes, as with any of the Prettier options, you definitely might want to customize based on the type of project.

So much bike-shedding going on here...
Please just give us a way to configure this.
Make the default value for the option whatever you want it to be - but please respect that not everyone will agree with it, and that people need choice.

@suchipi We just need a way to configure this. We want to choose whether or not we want this option,

We switched to use prettier a couple months ago, and it has been great so far. But if there is no way to use prettier with decorators on the same line as the variable (we use both mobx and angular in our project) then we are just going to not use prettier. Hard pass on this issue for us.

I agree. The newlines are ridiculous. Please change back.

Its a nice thought, but there are plenty of cases where forcing a newline after every decorator will make the code much less readable. Don't do it!

I love using prettier, but I really think moving decorators onto their own line is not a good solution (unless there are multiple of course).

Personally I would like to see it as a configuration option, offering:

"newline"
"sameline"
"ignore"

... or something to that effect. The default could be newline (much like now), but I also think for the newline option, there needs to be a space between each decorated property - seeing a big clump of properties with decorators on multiple lines makes my head hurt!

In terms of providing options, we could add the three suggested (inline, newline, leave unchanged) but has anyone considered having the option being a list of decorators that people want to have inline, on a newline, or left unchanged. That might let people who use Angular and TypeORM have one Prettier setting for their codebase.

Also, do people want different settings for class members and class functions? It seems having function decorators on a seperate line is much more liked than member decorators on their own line.

but has anyone considered having the option being a list of decorators that people want to have inline, on a newline, or left unchanged.

@aboyton Do you have any proof that people who might want this behavior even exist? See my other comment.

As an Angular developer, I ran into this issue. I’ll second the suggestion by @aboyton that it makes sense to have different behaviors for class properties vs methods. In my code, I might have something like:

@Input() label: string;
@Output() click = new EventEmitter();

@Input()
set id(value: number){
  this._id = value;
}

In other words, decorators on the same line for properties but not functions.

Also, see the TC39 proposal for decorators: https://github.com/tc39/proposal-decorators#decorators.

Note that while they have decorators that apply to methods on separate lines, the first decorator, one that applies to a property, is inlined:

 @observed #x = 0;

Would this be a decent place to draw a line between inlining vs newlines for everyone, so that we could have one consistent rule with no options?

@aboyton & @karptonite I agree about inline for properties, and new line for functions - this makes a lot of sense.

If we can't have it configurable, then this is the next best option I believe

@acidduckling I actually think this might be better than having it configurable. It is best to avoid adding option when at all possible.

Yeah, keep it simple.

I'd certainly be very happy with this approach! if a single decorator
property is one line, and function/method decorator is multi line by
default, then this is exactly how I'd want it anyway.

The rest of us won't be happy. I just can't agree that mixed styles look good or are easier to read. In my opinion it's only acceptable for demo code.

No Good:
image

Consistent and easier to read:
image

It sounds like the only consternation is the inline decorators for properties for those that don't comment code, or use code generators. It sounds like the only way to make everyone happy is to create an option to inline property decorators for those folks and let them deal with the legibility problems. Or allow plugins so that they can write their own prettier rule to inline to their hearts content. The default should be to put them on separate lines with a empty line before the first decorator and the previous block (except for comments).

@babeal well, I guess the one thing I think most (maybe all?) agree on is an empty line after a decoratated item should be implemented at the very least?

Yes I absolutely agree that run-on decorators are not good and need an empty line before.

If a decorator is not inlined, I agree that an empty line makes sense, but if they are inlined, the empty line may not be desirable.

It seems like the viable solutions so far are:

  1. Just leave decorators where they are.
  2. Inline for single, no-argument decorator with properties, otherwise separate line.
  3. Add an option to opt into solution 2, otherwise separate line for everything.
  4. Add an option to opt into solution 2 for a list of decorators (e.g., observable, input), otherwise separate line for everything.
  5. Add an option that lets you choose (1) leave decorators as is, (2) inline simple decorators for all properties, (3) inline simple decorators for properties for a list of decorators.

In all cases where decorators are not inlined, there shall be an empty line before the decorator and after the property, function, method, class, etc.

@devuxer I'd revise option 2 (and those that inherit from it): "Inline for single decorator with properties when it can fit on one line". I don't think that "no-argument" should be a part of that. An example from my Angular app:

 @Input('lrParent') parent: ApiItem;

I'd suggest that 4 (using a list of decorators) is messy, and probably not a good option.

@karptonite, The reason it might be useful to have a list of decorators is that some projects may use a combination of libraries that have different conventions regarding whether decorators should be inline or on a separate line (e.g., inline for MobX and separate for TypeORM). This may not be a good enough reason to incur the cost of the feature, but I just wanted it to be clear why it was proposed.

What is the status of this? Is there going to be an option? We would like to use prettier for our project but would like the option to keep decorators on the same line for variables.

I have to say that I whole heartedly disagree with @babeal! Decorators for variables look so much better on the same line. For functions, class, etc they should be above.

@dmarg They look horrible, and are difficult to read especially when you have some variables with single decorators that are inlined and some with multiple that are put on separate lines. Only does it work when all decorators are single, same named and variables are missing accessibility modifiers (which should be banned), and missing comments.

@babeal that is your opinion. I get that prettier is opinionated but there should be an option. It seems like there is a difference of opinion here and you are just one of the loud ones but maybe in the small minority.

//This looks better to me
@Decorator userName: string = '';

// this does not look as good
@Decorator
userName: string = '';

However for function calls I believe it is the opposite...

//This looks better to me
@Decorator 
getUserName(): string {
  return userName;
}

// this does not look as good
@Decorator getUserName(): string {
  return userName;
}

Again, these are all opinions. Which is why I think Prettier should include an option. There was talk about an option in another thread but it's unclear where the status of that is. I feel like Prettier could kind of move toward the way of TSLint. Have a recommended set but then have more options to override. Different people and teams want different prettierness and that's ok.

With all that being said there definitely should be a new line inserted after if Prettier converts a single to multiline to separate the decorators.

@vjeux This issue has been marked "needs discussion". I suggest that it has now had plenty, and it is time to move forward. I'm tagging you because you have the authority to make a call here--sorry if I'm overstepping.

Based on the discussion, it seems that in the current state, mobx and Angular developers will have to lock to 1.13. A few path forward are summarized above by @devuxer.

Are any of these options that make enough sense that we can settle on one, and someone can start on a pull request?

I am speaking for all the people that voted to have it changed this way, which is documented in the previous issue. They aren’t here since it works the way they want. I’m not trying to be rude in anyway, just want to provide representation :). I would be fine with an option as it is a preference.

In your first example, when you add the public accessibility modifier, does that make a difference for you? I spend most of my time daily code reviewing over many different languages and implicit accessibility modifiers causes extra time when I read the line to remind myself what the default is. It’s minuscule, but it adds up throughout the day so I prefer elements like this to be explicit. Once the line gets extended it just adds to the cognitive delay. It is completely subjective, like my inability to use dark ui’s when reading code. Speed in recognition and interpretation is extremely important in what I do daily so every little bit counts.

Though prettier isn’t perfect for this as it really messes up long generics and rxjs observables but at least it’s better than some of the special layouts our developers have cooked up.

@karptonite I'm not using Angular nor mobx and I tried downgrading to prettier 1.13 and it still did the multiline on all of the decorators.

Again, I feel like the option would be the best solution and don't understand why some are against adding the option.

@babeal since you are speaking for the users who voted to have it this way, perhaps you can address the fact that the current state goes contrary to the decorator proposal, here:
https://github.com/tc39/proposal-decorators#decorators
Note that they show property decorators (only) inlined.

@dmarg as for why some are against adding an option, prettier is by design an opinionated formatter, and seems only to offer options in rare cases where the community is so divided that there seems to be no other, uh... option. I'm not sure why downgrading to 1.13 didn't cause your decorators to be inlined; I'm locked in to 1.13.7, and my decorators are definitely inlined.

@karptonite I was using it on a TS file and it did not inline the decorators... just removed prettier and then reinstalled it and locked it again to 1.13.7 and it appears that it is ad hearing to the TC39 proposal which is what I like.

I do think there should be an option. I hear you on that prettier is opinionated and we want to reserve options for contentious items, but I feel that this maybe one of those times to use an option.

I'm actually neutral on whether we should add an option, as long as decorators can be inlined for properties. The reason I'm suggesting option 2 as a formatting that should make most users happy is that the prettier team leans strongly against options, and I don't want to give the impression that we are saying "it's an option or nothing" and have the response be, "then nothing." I would argue that inlining property decorators should be the default (in line with the TC39 proposal), and that those who want them on a separate line should be requesting an option.

@karptonite just because the syntax of the language supports it doesn’t mean it’s good code which is why we have tools like tslint, resharper, fxcop, etc. There is nothing in the proposal that suggests any recommended style.

I'm gonna lock this and limit discussion to contributors. I don't know what the solution will be but I think we've covered a lot of ground at this point and we understand the problem space and the limitations of each potential solution.

Adding the “bug” label to make it more clear that we intend to improve this (but haven’t decided exactly how yet).

This issue has now been open for two months, along with #1974 and #5088. We have found neither a good heuristic nor options that would work for all use cases.

Prettier keeps object literals multiline if they were written that way due to all the different things objects are used for, and we couldn't find a good heuristic for all cases. Looks like decorators are used for a lot of different things too.

After some internal discussion it was decided to take the same approach with decorators as with objects. While this is not ideal, we need to unblock this (while still being open for better solutions in the future).

We're open to a PR implementing the following:

  • If all decorators for a thing (property, function, class, etc) are written inline:

    • If they fit, keep them that way.

    • Otherwise go multiline.

  • If at least one decorator for a thing is on its own line (or spans multiple lines itself), go multiline (even if they would fit inline).
  • In other words, decorators will be either all on separate lines, or all inline.
  • Note that the decorators for one property of a class can be multiline, while another property on the same class can be singleline.
  • There might be some edge case I didn't think of, but we'll find out when a PR is made.

Hope I didn't miss anything.

I agree with the following:

  • If all decorators for a thing are written inline:

    • If they fit, keep them that way.
    • Otherwise go multiline.
  • If at least one decorator for a thing is on its own line (or spans multiple lines itself), go multiline (even if they would fit inline).
  • In other words, decorators will be either all on separate lines, or all inline.
  • There might be some edge case I didn't think of, but we'll find out when a PR is made.

But I would add one more:

  • If one or more decorators is on a separate line from the thing, ensure there is at least one line break before the first decorator and at least one line break after the thing.

Example:

// before
@decorator1
thing1
@decorator2
thing2
@decorator3
thing3

// after
@decorator1
thing1

@decorator2
thing2

@decorator3
thing3

I see what you mean, but let's wait doing the blank lines stuff for a later PR to keep things moving.

Fair enough. Should a separate issue be created for this feature?

Yep, I've seen those comments, but I still feel the same about this feature. I'll stand by until your proposal is done, though.

In case you wonder why this is closed it looks like is now fixed and will respect user formatting (inline vs new line) in class properties and also methods which I think was the correct approach, thanks!

So if it shows in a new line and you want it inline you just need to remove the new line.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mafredri picture mafredri  Â·  3Comments

grgur picture grgur  Â·  3Comments

n1k0 picture n1k0  Â·  3Comments

ikatyang picture ikatyang  Â·  3Comments

brigand picture brigand  Â·  3Comments