Implement fixer for ordering import declarations #1657
Conversation
6e79fac to
333ac08
Compare
There was a problem hiding this comment.
Tbh, it's hard to review all the logic going on here (not that it's that complicated, just hard to do in my head haha.) I'd recommend doing the following to make sure it works well:
Grab a few large TS codebases with unordered imports. Run this on them and make sure there aren't any runtime errors and that the results look good. If they do look good, we should be able to go ahead and merge this. If we find any errors, we can add those as test cases and get them fixed
| @@ -109,6 +109,19 @@ function findUnsortedPair(xs: ts.Node[], transform: (x: string) => string): [ts. | |||
| return null; | |||
| } | |||
|
|
|||
| function sortByKey<T>(xs: T[], getSortKeyCallback: (x: T) => string): T[] { | |||
There was a problem hiding this comment.
i'd recommend just calling this getSortKey, since it returns a string and not a function
| } else if (transformedA < transformedB) { | ||
| return -1; | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
Trivia: all these comparisons can be replaced with return transformedA.localeCompare(transformedB). it is slower though, and mainly only important if you're comparing non-english strings
There was a problem hiding this comment.
Yeah, I used localeCompare, but it ignores casing, so it changes the behavior from what it was before
There was a problem hiding this comment.
In hindsight, beyond performance concerns, you might be better off doing things this way anyways. I believe localeCompare could sort strings differently depending on the users locale and then we wouldn't have a deterministic way of sorting things, which would be bad
| node: ts.ImportDeclaration; | ||
| nodeStartOffset: number; // start position of node within source file | ||
| text: string; | ||
| sourceFile: string; |
There was a problem hiding this comment.
I'd recommend calling this something like sourcePath or just source, ad it doesn't always refer to a file and getSourceFile/SourceFile is used by TS with a different meaning in TSLint
Strip outer quotes before sorting Fix replacement of import statement if length changes
PR checklist
need a place to list fixers in docs
What changes did you make?
Auto-fixer for import declarations, and the named declarations inside of them.
Is there anything you'd like reviewers to focus on?
(optional)