add rule jsx-sort-props to order props alphabetically, optionally by type #97
Conversation
There was a problem hiding this comment.
Similar comments as I wrote in palantir/tslint#2831 (comment)...
I don't want to start introducing conflicting rules here without a good reason, so would prefer to avoid adding "shorthand" and "longhand" options.
As for "spread", there's no way to statically know the members of an object being spread, so I don't think we can safely suggest the correct sorting position of a spread. Let's just keep this rule simple and do alphabetic sorting.
I'll take another look when this PR has been slimmed down.
src/rules/jsxSortPropsRule.ts
Outdated
| type ORDER_OPTION = "spread" | "shorthand" | "longhand"; | ||
|
|
||
| enum KEY_ORDER { | ||
| OPTION_ORDER_SPREAD, |
There was a problem hiding this comment.
consistent 4-space indentation please
src/rules/jsxSortPropsRule.ts
Outdated
| Possible settings are: | ||
|
|
||
| * \`"${OPTION_ORDER_SPREAD}"\`: Spread assignments \`{...props}\`. | ||
| * \`"${OPTION_ORDER_SHORTHAND}"\`: Shorthand properties \`{ isError, isValid }\`. |
There was a problem hiding this comment.
shorthand conflicts with jsx-boolean-value: true, which we enable by default... https://github.com/palantir/tslint-react/blob/master/tslint-react.json#L5
|
Will address based on the outcome of my reply. Spread would no longer be a part of this change, and other than adding alpha-sorting, it would simply allow for ensuring shorthand come first and we can alpha-sort within those groupings. |
…horthand-first approach
|
Updated based on the same approach I took here: palantir/tslint#2831 (comment) |
|
Looking forward to this feature. @adidahiya , did the modifications made by @SteveByerly address your concerns? |
|
ping @palantir/tslint |
|
Not sure what the issue is with the builds here, but I'd guess @SteveByerly may need to push once more to have the tests run (and hopefully pass). If they pass, I'd be happy to merge this in. |
|
Hey @adidahiya! As I can see, the requested changes already implemented. Could you please review the PR? |
|
Any updates on this one @johnwiseheart ? |
|
Will it be possible to automatically fix unsorted props with tslint --fix? |
|
That would be fantastic! |
|
closing as stale, sorry it took so long to review @SteveByerly. let us know if it's ok for someone to take take your code and make a new PR. |
|
Will this ever be picked up again? |
Addresses #8
Also see similar update in tslint:
palantir/tslint#2831
CHANGELOG.md entry:
[new-rule]
jsx-sort-props: Sort props alphabetically and optionally by type (spread, shorthand, longhand)