Fixed an issue with type-only import promoting #55365
Conversation
typescript-bot
added
the
For Uncommitted Bug
| @@ -1329,15 +1329,17 @@ function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaratio | |||
| if (aliasDeclaration.isTypeOnly) { | |||
| const sortKind = OrganizeImports.detectImportSpecifierSorting(aliasDeclaration.parent.elements, preferences); | |||
| if (aliasDeclaration.parent.elements.length > 1 && sortKind) { | |||
| changes.delete(sourceFile, aliasDeclaration); | |||
| const newSpecifier = factory.updateImportSpecifier(aliasDeclaration, /*isTypeOnly*/ false, aliasDeclaration.propertyName, aliasDeclaration.name); | |||
| const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); | |||
There was a problem hiding this comment.
I think that there is an argument to be made here that this codefix shouldn't change the position of the promoted import. This is a separate action. I left this as-is though, as I assume that this was implemented this way for a reason.
There was a problem hiding this comment.
I suppose it might be annoying to users if they have their imports already sorted such that type-only imports are grouped separately from regular ones; promoting a type import will leave it in the wrong position in the list. Particularly if the user is relying on auto-imports which automatically puts them in the correct place, it'd be frustrating to have this one code fix that requires triggering Organize Imports manually when in every other case it's automatic.
There was a problem hiding this comment.
The reason that it moves in the list currently is that TS's organize/auto-imports always sorts type imports at end, but otherwise they're in order. Of course, via #55269 and #52820, this will become even more configurable. So effectively, any operation that does anything to imports (adding, removing, changing modifiers) has to go through some sort of sorting code to determine whether or not the user opted into sorting something in a particular way. Thankfully, the main goal of that configurability is enabling people to stick type imports inline such that these modifier changes don't affect sorting at all, which is nice.
There was a problem hiding this comment.
So effectively, any operation that does anything to imports (adding, removing, changing modifiers) has to go through some sort of sorting code to determine whether or not the user opted into sorting something in a particular way.
Do you happen to know if renaming exported symbols also sorts the relevant imports that use them?
There was a problem hiding this comment.
I'm pretty sure it doesn't, no; my understanding is that rename is more of an "in place transform" and it doesn't even check that the name you're giving it is syntactically valid.
Andarist
force-pushed
the
fix/codefix-promote-type-only
branch
from
c8083db to
e0c8912
Compare
typescript-bot
added
For Milestone Bug
fixes #55363