feat: add organizeImportsMode option#140
feat: add organizeImportsMode option#140oodzchen wants to merge 2 commits intosimonhaenisch:masterfrom
organizeImportsMode option#140Conversation
simonhaenisch
left a comment
There was a problem hiding this comment.
Hey thanks a lot for the PR 👏
Looks good to me overall, just a couple minor questions/feedback (:
lib/organize.js
Outdated
| filepath = 'file.ts', | ||
| organizeImportsSkipDestructiveCodeActions, | ||
| // @ts-ignore | ||
| organizeImportsMode = 'All', |
There was a problem hiding this comment.
Do you really need to set a default value here, since we're already setting the default in the plugin options?
Also what is the @ts-ignore ignoring? 🙃
There was a problem hiding this comment.
Since the default value is unnecessary here, this one could be delete.
lib/organize.js
Outdated
| { | ||
| type: 'file', | ||
| fileName: filepath, | ||
| skipDestructiveCodeActions: organizeImportsSkipDestructiveCodeActions, |
There was a problem hiding this comment.
| skipDestructiveCodeActions: organizeImportsSkipDestructiveCodeActions, |
I assume we can skip this now that we set mode based on it, since the property is deprecated anyway.
| type: 'file', | ||
| fileName: filepath, | ||
| skipDestructiveCodeActions: organizeImportsSkipDestructiveCodeActions, | ||
| // @ts-ignore |
There was a problem hiding this comment.
Same here, why a ts-ignore?
There was a problem hiding this comment.
It's the problem of use enum type in javascript code, since the languageService.organizeImports only accept mode type as OrganizeImportsMode, we must use something like OrganizeImportsMode.All to configure it, and it was imported from the typescript code, that's not a good idea. I couldn't find any other way to solve this problem, the simple way is just ignore it.
There was a problem hiding this comment.
ok we can just type cast it instead. i'll try to finish this PR soon, sorry somehow was too busy lately 🫣
| @@ -1,9 +1,9 @@ | |||
| declare module 'prettier' { | |||
| interface Options { | |||
| organizeImportsSkipDestructiveCodeActions?: boolean; | |||
There was a problem hiding this comment.
Sry i didn't mean dropping the deprecated option entirely. just when calling languageService.organizeImports, to only pass mode there, like
languageService.organizeImports(
// ...
mode: organizeImportsMode ?? (organizeImportsSkipDestructiveCodeActions ? 'SortAndCombine' : undefined),
// ...
)
I've try to use this plugin to auto remove unused imports, but it always sort the imports at the same time, which is not what I want. So I add the
organizeImportsModeoption to accomplish it, you can configAll、SortAndCombineorRemoveUnusedto determine how to organize the imports, and also, it's followed the latest typescript updates.