sveltejs / prettier-plugin-svelte

Format your svelte components using prettier.
MIT License
737 stars 97 forks source link

[feat] Sorting component or element props #161

Open dkzlv opened 3 years ago

dkzlv commented 3 years ago

Is your feature request related to a problem? Please describe. To make it clean I always try to sort the props, event handlers, actions and so on in a consistent way. Manually, of course. Wouldn't it be cool to do it automatically?

Describe the solution you'd like It would be super nice if this plugin could sort the props automatically with some rules, like: class, shorthand conditional class, conditional class, shorthand props, key props, actions without params, actions with params, forwarding events, handling events and, eventually, CSS vars. Maybe it would be cool to also sort them alphabetically as well. It would also be nice if it automatically grouped all these properties with an extra newline (now it removes all of them).

It would turn a messy element (real-world example, btw), like

<input
  class="input"
  class:is-success={field.isBlurred && field.isValid}
  type={field.type}
  {id}
  value={field.inputValue || ''}
  disabled={field.disabled || $formStore.formDisabled}
  placeholder={field.placeholder}
  readonly={field.readonly}
  aria-label={field.label}
  autocomplete={field.enableAutocomplete ? '' : 'off'}
  on:input={onInput}
  use:clearOnEscape
  class:is-danger={field.isBlurred && !field.isValid}
  on:blur />

into this beautiful thing:

<input
  class="input"
  class:is-success={field.isBlurred && field.isValid}
  class:is-danger={field.isBlurred && !field.isValid}

  {id}
  autocomplete={field.enableAutocomplete ? '' : 'off'}
  aria-label={field.label}
  disabled={field.disabled || $formStore.formDisabled}
  placeholder={field.placeholder}
  readonly={field.readonly}
  type={field.type}
  value={field.inputValue || ''}

  use:clearOnEscape

  on:blur 
  on:input={onInput} />

Describe alternatives you've considered Just do it manually.

How important is this feature to you? Not that much, but would be a very cool addition!

Additional context

  1. I'm not sure it is in the scope of the project at all
  2. I'm not sure if changing the order doesn't have any side effects. Afaik, it is common to make templates pure-ish so they don't change the state, but a user can still call some functions that make the order of props kind if important. I would think it is a very bad pattern but still it's not prohibited.
ehrencrona commented 3 years ago

Personally I try to order attributes in some sort of logical order. What that is depends a lot on the context. I also order them so they "look nice", i.e. line length is similar etc. I don't think there's any way we could build some kind of algorithm for ordering them that everyone would happy with.

pngwn commented 3 years ago

I don’t really order them logically. It is always around readability. So events then directives then props/ attrs then shorthand props / attrs. I wouldn’t be against this addition as long as it could be disabled. It should not be configurable though.

dummdidumm commented 3 years ago

Funny, I would also order them around groups, but my order would be attrs/props/shorthandprops/directives/events. I think this is the main problem of this setting: If we add one, most of the people will say it doesn't fit their expectations.

pngwn commented 3 years ago

It is okay to be opinionated as long as those opinions are correct, which is to say, they agree with mine. 👹

I personally think all component should look like upside down Christmas tree.

dkzlv commented 3 years ago

@pngwn

It should not be configurable though.

Why? We already have an option to sort the order of component's content (script-styles-template). I thought there should just be a sane default to meet the spirit of the original Prettier :)

Personally, I don't have any opinion on the order itself. I just want it to be predictable, consistent and completely automatic. It is so satisfying to write some badly styled code and see how the machine sorts it out on save 🤩

dkzlv commented 3 years ago

Implementation detail: if you use any spread, it should disable the auto sorting for this element so the overwrite logic is not changed.

dkzlv commented 3 years ago

Another thing hit my mind, that maybe Svelte leaves the order of conditional classes intact, so

<div class='blah' class:changes-blah={bool} />

and

<div class:changes-blah={bool} class='blah' />

would result in different DOM result, so, probably, class: and class properties should also be left as is.

gustavopch commented 2 years ago

Personally I try to order attributes in some sort of logical order. What that is depends a lot on the context. I also order them so they "look nice", i.e. line length is similar etc. I don't think there's any way we could build some kind of algorithm for ordering them that everyone would happy with.

I think that's against the very reason why Prettier exists. It's not meant to please everyone's particular tastes, but to ensure consistency above everything else and reduce how many code style choices people have to take.

dummdidumm commented 2 years ago

I don't think we can make this an option, because the order matters and there's no good way to tell the user's intent - see the last section in https://svelte.dev/docs#bind_element_property .