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

Make all jsx-prop-sort lints fixable #2250

Merged
merged 2 commits into from Apr 25, 2019
Merged

Make all jsx-prop-sort lints fixable #2250

merged 2 commits into from Apr 25, 2019

Conversation

guliashvili
Copy link
Contributor

@guliashvili guliashvili commented Apr 24, 2019

Not all the jsx-prop-sort lints were fixable.
There were cases when existing fix would cause other lints to trigger.
For example, in case when normal props were not sorted but callback functions were in the end(As they are supposed to be), sorting lint fix would mix callbacks props in between the other props.

Case:
ignoreCase = false;
callbacksLast = true;
shorthandFirst = false;
shorthandLast = false;
noSortAlphabetically = false;
reservedFirst = false;
--
<div a z onClick=y />

Existing lint fix would transform it to
<div a onClick=y z />

However, this also triggers another lint about callbacksLast.

Updated the tests accordingly.

Not all the prop sort lints were fixable.
There were cases when fix would mess other things up.
For example, in case when normal props were not sorted but callback functions were in the end(As they are supposed to be), sorting lint fix would mix oncallback props in between the other props.

Updated the tests accordingly.
lib/rules/jsx-sort-props.js Show resolved Hide resolved
lib/rules/jsx-sort-props.js Show resolved Hide resolved
lib/rules/jsx-sort-props.js Outdated Show resolved Hide resolved
lib/rules/jsx-sort-props.js Outdated Show resolved Hide resolved
lib/rules/jsx-sort-props.js Outdated Show resolved Hide resolved
lib/rules/jsx-sort-props.js Outdated Show resolved Hide resolved
@guliashvili
Copy link
Contributor Author

@ljharb Thank you for so fast review Jordan.
I simplified code a bit by removing unnecessary if branches. Please let me know if there is anything else I can change.

@ljharb ljharb merged commit d5d2e82 into jsx-eslint:master Apr 25, 2019
@guliashvili
Copy link
Contributor Author

@ljharb @yannickcr
Looks like before, new tags were cut frequently but not anymore. Is there any bug or something stopping you from cutting the new tag(I can work on that also).

Would appreciate if you cut the new tag.

@ljharb
Copy link
Member

ljharb commented May 1, 2019

Yes, tests are failing on master right now; see #2253

@ljharb
Copy link
Member

ljharb commented May 7, 2019

v7.13.0 is released.

@guliashvili
Copy link
Contributor Author

Thank you @ljharb . Appreciated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants