Disable alphabetic sorting of object keys

[miklschmidt]

So far the only pro i've heard for this is that keys are easier to find when they're alphabetically sorted. I think this is an invalid pro because to make use of alphabetic sorting, you need to know the name of the key you're looking for. If you already know the name, ctrl/cmd+f is your friend.

Object key sorting:
pros:

• Consistency. Very consistent albeit arbitrary ordering of member names.
• to work around issues such as width and height grouping you can create special rules, although that would require a case by case rule addition, potentially a lot of work.

cons:

• Doesn't work with css in JS rule overriding, where the order has meaning. For example, i can't override padding with a paddingBottom because the wrong order is enforced.
• You can't group keys that logically belong together, such as width and height, or isModalShown and toggleModal.
• If you want to achieve the previous, you're forced to write less readable key names, such as modalIsShown, modalToggle, modalShow. Yoda style. This results in less readable code.

Possible solution to achieve non-arbitrary consistency:

• Consistent ordering can be achieved through a less intrusive way and on a case-by-case basis (i.e. logical ordering for the most used CSS keys would be awesome)

Feel free to add pros/cons to the list!

Vote 👍 for removal, 👎 for keeping.

[andsens]

fwiw.

pros:

• a very consistent albeit arbitrary ordering of member names
• to work around issues such as width and height grouping you can create special rules

cons:

• consistent ordering can be achieved through a less intrusive way and on a case-by-case basis (i.e. logical ordering for the most used CSS keys would be awesome)

[miklschmidt]

Thanks i'll add those!

[bondo]

I like that there's a single correct way to order both objects and members, but I don't feel strongly that we have to enforce it. The cons probably outweigh the pros here.
But I think it's important to enforce the member order wrt. access level, and we should probably keep doing some special case for React components.

[miklschmidt]

@andsens in case of special rules, you'll have to add a new rule on each case you encounter, no?

[andsens]

@miklschmidt, yes. That goes both for the special rules mentioned in the pros as well as the cons.

[miklschmidt]

@bondo Totally agree with the member ordering, ordering by access level and component lifecycle methods is great. I like enforcing order that makes sense.

[miklschmidt]

@andsens

consistent ordering can be achieved through a less intrusive way and on a case-by-case basis (i.e. logical ordering for the most used CSS keys would be awesome)

Is kind of a strangely worded con. Is it a con that there's another solution to a part of the problem?

[andsens]

Is kind of a strangely worded con. Is it a con that there's another solution to a part of the problem?

Well, I'd say pretty much the whole problem. As in, instead of creating an all-encompassing rule and the whittling it down through special cases, build up a set of rules instead that eventually cover every aspect.

[miklschmidt]

@andsens oh yea i agree with that, but is that a con? Isn't that more of a "how we can do it differently", or a "better solution".. I'll add that.

[kastermester]

I think there's several rules in place here - and we should split this issue in two.

There's object member ordering and class member ordering. These are two quite different things - as one can affect the semantics of the program and one cannot.

With that said - I can follow the argument for removing the rule around object member orderings - these rarely tend to be so large that there's an actual advantage, whereas classes do grow quite large and figuring out where the "correct" place to add a new member can be quite tedious.

I think it is important to note here that, as long as there's a rule in place, to order by alphabetical names, one should not simply rename something to group it together - this is not the intent behind these rules - you're not supposed to do something to get to what you tried to do that the linter told you was not the way. The linter tells you to move a member - not rename it.

The goal around trying to find a rule that "makes sense" or "just works" around ordering of stuff (that groups things together) is a noble one, but I don't see it happening. There's simply way too many special cases to handle if that is what you want to do. And what is the plan here? Creating a rule that says you have to group width and height together, not caring about anything else?

@miklschmidt Can you change this issue to be solely related to object member ordering? If you feel strongly about class member orderings then also create an issue for that? (which btw, there's a ton of options of how to order class members, see the tslint docs on class member orderings, it's not a simple yes/no).

As stated earlier - I'm fine with removing the rule on object member orderings. Personally I do prefer not having to think about where to put my stuff and knowing where I should find it again - as I would much rather focus my energy on actual coding - but if the rest of you feel strongly about this there's not that much difference here and that can be worked out. Do remember though - if the goal is to create a rule about logical grouping, then you need to come up with an actual plan for implementing that. Simply saying "add stuff one by one" is not enough. Do we add stuff that we have rules for in random places in the object? Top, or bottom? Creating such a rule set is a huge task - and would probably not do down well.

There's also always the option of trying to solve the CSS issues in a special case basis (where we could create rules for logical grouping - assuming one would come up with a complete set of rules). That is a tedious, but doable task.

[miklschmidt]

@kastermester

Can you change this issue to be solely related to object member ordering? If you feel strongly about class member orderings then also create an issue for that?

Absolutely, will do!

Personally I do prefer not having to think about where to put my stuff and knowing where I should find it again

You should think about where you put your stuff, that's what makes the difference between good readable code and less readable code. Also as long as we don't have an auto fix for alphabetic sorting, you have to think way harder than if you didn't have to alphabetically sort everything. We can make a style guide for this if you think adding rules for it is too much work. We could do something like this:

// REACT LIFECYCLE
componentDidMount: () {}
shouldComponentUpdate: () {}

// MODAL
toggleModal: () {}
showModal: () {}

// POPOVER
togglePopover: () {}
showPopover: () {}

Which would solve that problem, i know it's practically impossible to enforce programatically, but again, i don't think we should choose a lesser solution because our tooling says no. Randomly forcing an arbitrary sorting that doesn't yield a net benefit is not really fixing anything, other than consistency.

Personally i'd rather have inconsistent readable code than consistent unreadable code, if you catch my drift. I totally agree with enforcing things that make the code more readable, such as enforcing react lifecycle methods in the top of the class, access level sort order and so on, because those are specific logical groupings, we should have more of those, and less of the arbitrary stuff.

[kastermester]

I guess I'm just not that convinced that we will end up with a better grouping when it is not always clear that there is one.

I do like the idea around possibly using comments to create such groupings - which is also an option that we could explore. To have a default that will do alphabetical sorting and then allow creating separate groups after the default (to ie. group all stuff having to do with width together).

[miklschmidt]

To have a default that will do alphabetical sorting and then allow creating separate groups after the default (to ie. group all stuff having to do with width together)

Can we do that? I'd be okay with that compromise!

[kastermester]

It'd take some work, but I can't see why not.

[miklschmidt]

@andsens @bondo what do you think?

[kastermester]

I'm thinking something like the following:

• Objects without groups are alphabetical ordered.
• Some way of noting a group in a comment (possibly ordering the groups alphabetically as well). Exact syntax we can always discuss.
• In the groups, we also use alphabetical ordering.
• Grouped members must appear after non grouped members.

Does this sound reasonable to you? Or is there something you'd like to see changed?

[miklschmidt]

What's "Defaults"?, rest sounds good!

[kastermester]

Ah defaults was a bad name, (will edit) meant ungrouped.

Here's some examples:

Default/ungrouped:

{
  a: 1,
  b: 2,
  c: 3,
}

Groups

{
   // My group
  b: 1,

  // My other group
  a: 1
}

Mixed groups and ungrouped/defaults

{
  c: 1,

  // My group
  b: 2,

  // My other group
  c: 3
}

[miklschmidt]

Ah yes! Perfect!

[miklschmidt]

Could we maybe make a "special group" called "CSS" that just disables the sorting completely until we have a working DSL or some better rules?

[kastermester]

Technically sure, although I'd probably call it something different.

The only issue worth mentioning here is that it'd only affect the object it is defined in (ie. not nested objects.). Also, it'd not be much different from simply disabling the rule and reenabling it.
There might be something we could do, but off the top of my head I'm not exactly sure how we could make something that would be better than what is already possible for this.

[miklschmidt]

Yea well you've got a point there! I'll just disable it for CSS for now.