-
Notifications
You must be signed in to change notification settings - Fork 885
Ordered-Imports Rule: Allow sorting by the trailing end of the module path #3178
Conversation
Thanks for your interest in palantir/tslint, @berickson1! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
src/rules/orderedImportsRule.ts
Outdated
@@ -104,27 +116,39 @@ const TRANSFORMS = new Map<string, Transform>([ | |||
["case-insensitive", (x) => x.toLowerCase()], | |||
["lowercase-first", flipCase], | |||
["lowercase-last", (x) => x], | |||
["full-path", (x) => x], | |||
["module-name", (x) => { | |||
const splitIndex = x.lastIndexOf("/"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably handle imports from node_modules special. Otherwise sorting would be odd when using submodule imports or scoped modules:
import foo from "a/foo"
import bar from "b/bar"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajafff is there an easy way to tell when importing from node_modules vs. relative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look at noSubmoduleImportsRule.ts
it contains such a condition:
!(ts as any as {isExternalModuleNameRelative(m: string): boolean}).isExternalModuleNameRelative(expression.text) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good.
I won't judge if this new option makes sense / is needed, because I don't like this rule at all.
src/rules/orderedImportsRule.ts
Outdated
Possible values for \`"module-source-path"\` are: | ||
|
||
* \`"full-path'\`: Correct order is \`"./a/Foo"\`, \`"./b/baz"\`, \`"./c/Bar"\`. (This is the default.) | ||
* \`"module-name"\`: Correct order is \`"./c/Bar"\`, \`"./b/baz"\`, \`"./a/Foo"\`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer "full"
and "basename"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind adding this option, but please rename the option names to "full"
and "basename"
as @ajafff suggested.
@berickson1 also let me know if you have any issues signing the CLA |
Sorry for the CLA delay - I had to get my employer's blessing first |
Thanks @berickson1 |
PR checklist
Overview of change:
Add an additional argument to ordered-import rule allowing sorting by trailing end of path. This allows sorting modules with relative path by filename
CHANGELOG.md entry:
[enhancement] Allow sorting imports by trailing end of path