jsx-sort-props rule is not working as it should

[zalishchuk]

Rule configuration:

'react/jsx-sort-props': [
  'error',
  {
    noSortAlphabetically: false,
    ignoreCase: true,
    callbacksLast: false,
    shorthandLast: true,
    reservedFirst: true,
  },
],

As a result callbacksLast, shorthandLast options are overridden by noSortAlphabetically, so props are sorting only by alphabet ignoring shorthands at the end. Also, how can I reach this sort order:

• object spread props
• reserved props
• common props (html attributes, etc.)
• callback props
• shorthand/boolean props

Example:

<button
  {...rest}
  key={1}
  type="button"
  onClick={() => {}}
  disabled
/>

[ljharb]

It would be dangerous to enforce object spread props on any particular order, as you might intend props before it to be overridden, and props after it to override.

The collision between options sounds like it might be a bug.

[zalishchuk]

We can discard issue about object spread props, but what about noSortAlphabetically bug?

[ljharb]

What error do you get from your example?

[zalishchuk]

As I mentioned before noSortAlphabetically: false option overrides callbacksLast and shorthandLast, that's why props are sorted only by alphabet and shorthand prop is on the top, but it should be at the end of component props.

[ljharb]

In your example, the shorthand prop is on the bottom.

Can you provide example code and config, combined with actual and expected output?

[fleischie]

Hey I just stumbled over this as well. I figured I just add some requested data:

Example Code + Config

Code

import React from 'react';

export const Test = () => (
  <p z="test" a />
);

Config

{
  "root": true,
  "extends": ["eslint:recommended", "plugin:react/recommended"],
  "parser": "babel-eslint",
  "parserOptions": {
    "ecmaFeatures": {
      "experimentalObjectRestSpread": true,
      "jsx": true
    },
    "sourceType": "module"
  },
  "plugins": [
    "react"
  ],
  "rules": {
    "react/jsx-sort-props": [2, {
      "reservedFirst": true,
      "shorthandLast": true
    }
  }
}

Expected Output

Note: I run node_modules/.bin/eslint <file> to produce the following output.

Actual Output

/Users/kfle/Documents/Programming/work/OetkerDigital/backen/test.js
  4:15  error  Props should be sorted alphabetically  react/jsx-sort-props

✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

"Interesting" Observation

If I only supply the shorthandLast linting rule I get the expected result (also the expected output, if I incorrectly sort the shorthand prop before any others).

Example working config for only shorthand rule:

  ...
  "rules": {
    "react/jsx-sort-props": [2, {
      "shorthandLast": true
    }]
  }

[zalishchuk]

My own opinion, this rule should be remade to be similar to sort-comp.

[fleischie]

@zalishchuk can you elaborate a little on what you mean specifically? Do you mean the documentation, the rule's structure, the validation algorithm? I'm just curious...

[zalishchuk]

@fleischie I want the rule configuration to look like this.

"react/jsx-sort-props": ['error', {
  order: [
    'reserved',
    'everything-else',
    'callbacks',
    'shorthands',
  ]
}]

[fleischie]

Hmm, yes. And the ignoreCase and noSortAlphabetically would then be additional attributes to the options-object I guess? That would sound awesome indeed...

[fleischie]

Before I dive deeper into this issue code-wise, I still have some questions as this will be a breaking change (the configuration structure will drastically change):

• How would be a good way to get started on this?
• Should I first fix the colliding options?
• Should I directly break everything just for the sake of some awesom-o feature? (Probably not)
• Should I wait for a maintainer to give his consent/guidance on this? (Probably)

Any additional pointers will be gladly taken.

[ljharb]

We should avoid a breaking change.