Enforce props alphabetical sorting

[igmoweb]

Finding props sometimes in a React file is a bit difficult due to the random order. We could consider jsx-sort-props rule to automagically sort them alphabetically.

In the same line, we could also think about adding https://eslint.org/docs/rules/sort-imports to sort imports too.

[kadamwhite]

I'm 👎 on requiring alphabetization of component properties. We experimented with it at my last company, and it was a consistent distraction and annoyance for the majority of the team. While the intent is good, in practice it means that you usually can't group related props together, and spend more time (even with --fix) thinking about and updating prop order than you do actually writing useful code. It slowed down our component development noticeably to have to keep looking over our shoulder to make sure we weren't doing things wrong, and it polluted our lint warnings constantly while prototyping.

I'd also argue that this papers over the bigger issue of, if you have a component that includes so many properties you need to order them, then it should probably be split up into different components or you should use containers further down in the tree.

tl;dr, we tried it, tor three projects in a row it got cited as more trouble than it was worth. I'm in favor of alphabetical ordering of imports (Ryan's proposed ("absolute, then relative, then side effects, with a blank line between each'), and I'm fine with this being an optional rule we can add per-project, but I will do what I can to prevent jsx-sort-props from being adopted within the formal & official Human Made coding standards :)

[kadamwhite]

Broke import sorting out into a separate issue so we can discuss separately.

[igmoweb]

@kadamwhite Sounds a good reason to me. Thanks!

[mattheu]

I'm re-opening this issue because it is stated in our coding standards that we should have props in alphabetical order.

Personally I'm 👍 on enforcing the ordering, but i'm not too hung up either way. However I don't think it should really be in the written coding standards if we're not enforcing the rule in our linters. If its optional, may as well remove it! This has already caused some confusion for @BronsonQuick.

I think the rule for sorting HTML properties would be covered by react/jsx-sort-props, but we could add sort-keys too.

[kadamwhite]

I recognize that my comment above represents a personal bias, but it is a strong personal bias 😁, so I've proposed humanmade/Engineering#124 to bring the engineering guide in line with the discussion above.

[rmccue]

We had some pretty good discussion on https://github.com/humanmade/Engineering/pull/124 that didn't filter back here, but the ultimate text we ended up with was:

Consistent property ordering makes it easier to manage complex components. Properties should generally be sorted into the following groups, and ordered alphabetically within those groups:
Exercise discretion when alphabetising. Alphabetical ordering may not always be the best decision; for example, it may be preferable to keep related positional variables (e.g. height & width) grouped together for clarity.

This points towards a strong default for the ordering of properties in JSX, so we should have a rule that pushes us towards this, probably using react/sort-props.

Looks like the noSortAlphabetically option can be used when the strict ordering isn't desired. We can use inline comments for this which then also serves as an explicit indication that the non-sorted keys are intentional. This should preserve the grouping, which generally is always what we want.

For example:

// Sorted into groups instead
// eslint "react/sort-props": [ "error", "noSortAlphabetically" ]
return (
    <svg
        x={ 0 }
        y={ 0 }
        height={ 100 }
        width={ 0 }

(n.b. not tested that code)

[igmoweb]

I just filed a new PR here to add the rule: #195

[kadamwhite]

Resolved by adding this as a warning in #195