Skip to content
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

Fix reservedFirst for jsx-sort-props rule #1883

Merged
merged 2 commits into from Jul 16, 2018
Merged

Fix reservedFirst for jsx-sort-props rule #1883

merged 2 commits into from Jul 16, 2018

Conversation

fleischie
Copy link
Contributor

Hello,

this PR should (hopefully) fix #1692 .
Previously if reservedFirst was enabled, it would customly check for the sorting of the props, if the currently viewed items were of the same type (either both reserved or both unreserved) and if not otherwise set up (i.e. noSortAlphabetically was enabled).

This PR now adds test cases for the viewed case: reservedFirst and shorthandLast (I didn't add any more test cases, but can do so, if requested). Also it removes the special handling of the sorting but delegates to the rest of the handler to take care of any other options (e.g. shorthandLast, shorthandFirst, callbackLast, etc.). (Also there was a missing parameter in the shadowed alphabetically-sorting check, which I added).

Yeah. If you need/require anything else from this PR, hit me up and I'll add it. Additionally I hope it's ok, when I regularly rebase this onto master, if any other development is done (if not, let me know as well). 😄

Add tests, that surface the incorrect behavior of the combinations
`reservedFirst` and `shorthandLast` for the `jsx-sort-props` rule.
- Remove unneccessary handling of sort-checking in jsx props, as this is
  being handled later by the general case anyways.
- Fix erroneous call to `generateFixerFunction` in general
  sort-checking, which was missing the `reservedList` parameter.
@ljharb ljharb added the bug label Jul 16, 2018
@ljharb ljharb merged commit 0ebcb62 into jsx-eslint:master Jul 16, 2018
@fleischie fleischie deleted the jsx-sort-props/fix-reserved-first branch July 16, 2018 14:17
This was referenced Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

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