Skip to content

jsx-sort-props reservedFirst conflicts with callbacksLast #1632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jakezatecky opened this issue Jan 7, 2018 · 3 comments
Closed

jsx-sort-props reservedFirst conflicts with callbacksLast #1632

jakezatecky opened this issue Jan 7, 2018 · 3 comments

Comments

@jakezatecky
Copy link

jakezatecky commented Jan 7, 2018

It appears that PR #1340 introduced a change when reservedFirst is enabled such that it conflicts with the option callbacksLast. When this option is enabled, ESLint wants callbacks to be sorted alphabetically with respect to the rest of the properties. The following will incorrectly generate an error:

eslintrc:

{
  "rules": {
    "react/jsx-sort-props": ["error", {
      "callbacksLast": true,
      "reservedFirst": true
    }]
  }
}
<button
  className={className}
  disabled={disabled}
  type="button"
  onClick={this.onClick}
>
  Submit
</button>

Notice that onClick occurs after type, which should be the case given the rule configuration. However, the change in this PR made it to where the React plugin wants onClick to appear before type, effectively disabling the entire callbacksLast rule.

I have confirmed that reverting jsx-sort-props.js fixes this bug, but obviously that is not ideal because the alphabetic sorting fix in that PR is also desirable.

@jakezatecky
Copy link
Author

jakezatecky commented Jan 7, 2018

Offender appears to be the second conditional check at line 249.

jakezatecky added a commit to jakezatecky/react-dual-listbox that referenced this issue Jan 7, 2018
@jakezatecky jakezatecky changed the title jsx-sort-props requiredFirst conflicts with callbacksLast jsx-sort-props reservedFirst conflicts with callbacksLast Jan 7, 2018
@ljharb
Copy link
Member

ljharb commented Jan 7, 2018

@jakezatecky if you could provide a PR with a failing test case (or better, a fix!) that would be very appreciated.

kolpax pushed a commit to kolpax/eslint-plugin-react that referenced this issue Feb 12, 2018
ljharb pushed a commit to kolpax/eslint-plugin-react that referenced this issue Aug 11, 2021

Verified

This commit was signed with the committer’s verified signature.
ljharb Jordan Harband
…icts with callbacksLast

See jsx-eslint#1632.
ljharb pushed a commit to kolpax/eslint-plugin-react that referenced this issue Feb 4, 2022

Verified

This commit was signed with the committer’s verified signature.
ljharb Jordan Harband
…icts with callbacksLast

See jsx-eslint#1632.
@ljharb
Copy link
Member

ljharb commented Feb 4, 2022

Closed by #1632.

@ljharb ljharb closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants