-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
import/order
: alphabetize import type
above (or below) import
#2339
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
Comments
As an amusing side effect, this terrible monkey patch also appears to perfectly resolve #1881, since the alphabetization algorithm is slightly influenced by the appended text. const comparator = alphabetizeOptions.caseInsensitive
? (a, b) => sorterFn(`${a.value.toLowerCase()}|${a.node.importKind}`, `${b.value.toLowerCase()}|${b.node.importKind}`)
: (a, b) => sorterFn(`${a.value}|${a.node.importKind}`, `${b.value}|${b.node.importKind}`); |
@aaronadamsCA that type of comparison is what it used to do, and that caused bugs. That patch would break tests. |
I would expect type imports to always sort after value ones, since the types are far far less important and valuable. I agree that this plugin should have an opinion about which order it sorts those in. |
Note that I filed #2398 as a first attempt at solving this. |
…ts with same path based on their kind (`type`, `typeof`) Fixes import-js#2339 Co-authored-by: Aaron Adams <[email protected]> Co-authored-by: stropho <[email protected]>
…ts with same path based on their kind (`type`, `typeof`) Fixes import-js#2339 Co-authored-by: Aaron Adams <[email protected]> Co-authored-by: stropho <[email protected]>
This would be nice. The main reason we sort imports is to minimize merge conflicts, and I've noticed when sorting with this plugin that it has no opinion on the order between these two lines:
Both
simple-import-sort
and, as mentioned,@typescript-eslint/consistent-type-imports
placeimport type
(orexport type
) aboveimport
(orexport
) when the paths are identical; that seems like it could be a good precedent to follow.This might be as easy as concatenating
importKind
within the alphabetize comparator; I tried this with a local monkey patch and it seemed to do the trick.The text was updated successfully, but these errors were encountered: