Organize type imports #55269

iisaduan · 2023-08-04T16:54:10Z

Adds a configuration to change where named type imports are sorted. organizeImportsTypeOrder can be specified as "first", "last", or "inline", with default behavior being type imports are sorted last.
Autoimports will place the new named import at the end of the list of named imports unless the imports are sorted as specified
Fixes a bug in verifyImportFixAtPosition (function used for testing)

…ganize-type-imports

typescript-bot · 2023-08-04T16:54:24Z

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

typescript-bot · 2023-08-04T16:54:25Z

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

jakebailey · 2023-08-04T16:56:04Z

src/server/protocol.ts

+     * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if imports are
+     * type-only.
+     *
+     * Default: `inline`


The current default is effectively "last"; for backwards compat I'm thinking we should probaby keep that?

That makes sense to me, and is inline with the past changes to organize imports

Of course, changing the default to inline was what I was doing when I was attempting to make this change, was very helpful in debugging.

jakebailey · 2023-08-04T17:01:25Z

This same code should also be affecting auto-imports; do we have enough test coverage for that? I'm not seeing any of those tests change which is suspicious as either it means that it's not working, or we're missing tests for auto-imports with type imports.

tests/cases/fourslash/autoImportTypeImport1.ts

…ganize-type-imports

…rganize-type-imports

andrewbranch

Looks pretty good to me! Just a couple notes

src/services/organizeImports.ts

tests/cases/fourslash/autoImportTypeImport3.ts

tests/cases/fourslash/importNameCodeFix_importType10.ts

andrewbranch

Nice work!

andrewbranch · 2024-01-09T18:52:26Z

src/server/protocol.ts

@@ -3643,6 +3643,13 @@ export interface UserPreferences {
     * Default: `false`
     */
    readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
+    /**
+     * Indicates where named type-only imports should sort. "inline" sorts imports without regard to if the import is


When you add the VS Code option for this, make sure to say “type-only named imports” or “type-only import specifiers” since “type-only imports” could also refer to the relative order of these:

import type a from "./a"; import b from "./b";

which IIUC is unaffected by this preference.

jakebailey

Some of the orderings seem weird, but is that because they aren't sorted?

jakebailey · 2024-01-09T18:58:39Z

tests/cases/fourslash/autoImportTypeImport1.ts

+goTo.marker("");
+
+verify.importFixAtPosition([
+`import { A, D, type C, type B } from './foo';


Why does this one turn out to have C first? The next one doesn't which feels odd.

Since it's adding type B and it's determined that the types are unsorted (since it specified inline), it adds it on to the end. If the imports were unsorted before, it would always add onto the end, otherwise it might add the import somewhere weird in the middle, since we're not changing the order of the rest of the imports

tests/cases/fourslash/autoImportTypeImport1.ts

tests/cases/fourslash/autoImportTypeImport2.ts

jakebailey

I've stared at the tests and I think everything is in order.

Note that the other new import sorting options are not actually exposed in VS Code yet (you have to use the "unstable" field IIRC, see settings.template.json), so I don't think we need to do that quite yet.

One thing I'm not sure I'm seeing is detection, in that we detect that imports are sorted "inline" and then use it. Am I missing that (there's a lot of tests...) or is that coming later?

tests/cases/fourslash/autoImportTypeImport4.ts

iisaduan · 2024-01-09T19:47:21Z

@jakebailey

One thing I'm not sure I'm seeing is detection, in that we detect that imports are sorted "inline" and then use it. Am I missing that (there's a lot of tests...) or is that coming later?

Coming later!

iisaduan · 2024-01-09T19:48:23Z

@andrewbranch

Note that the other new import sorting options are not actually exposed in VS Code yet (you have to use the "unstable" field IIRC, see settings.template.json), so I don't think we need to do that quite yet.

is organizeImportsIgnoreCase exposed to VScode?

andrewbranch · 2024-01-09T19:52:37Z

I thought it was, but I guess it’s not.

iisaduan · 2024-01-09T23:12:38Z

Note that the other new import sorting options are not actually exposed in VS Code yet [...]

briefly discussed offline, and were reminded of #52115 (comment), so there's some more thinking to be done about how to expose the setting before we do

jakebailey · 2024-01-09T23:13:42Z

Yep, that's my understanding.

iisaduan added 4 commits August 3, 2023 12:19

add organizeImports option to sort types first, last, or inline

14afb68

Merge branch 'main' of https://github.com/iisaduan/TypeScript into or…

f44793d

…ganize-type-imports

update tests

e720377

change detection to account for type ordering

3652bce

typescript-bot assigned iisaduan Aug 4, 2023

typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Aug 4, 2023

jakebailey reviewed Aug 4, 2023

View reviewed changes

iisaduan requested review from andrewbranch and rbuckton August 4, 2023 19:06

add tests for autoimports

ee32e43

andrewbranch reviewed Aug 4, 2023

View reviewed changes

tests/cases/fourslash/autoImportTypeImport1.ts Outdated Show resolved Hide resolved

jakebailey mentioned this pull request Aug 14, 2023

Fixed an issue with type-only import promoting #55365

Merged

iisaduan added 8 commits September 11, 2023 15:09

Merge branch 'main' of https://github.com/iisaduan/TypeScript into or…

d47d1b6

…ganize-type-imports

update baselines

bb47232

Merge auto-import-type

44b07dd

change default order to "last"

4a83f8c

add types to detection, adjust tests accordingly

489e92a

fix potential organizeImports chaining in test calls

11929fe

Merge branch 'main' of https://github.com/microsoft/TypeScript into o…

63dcdb6

…rganize-type-imports

post-merge bug fix

4560a6b

iisaduan requested review from andrewbranch and jakebailey January 8, 2024 20:30

andrewbranch reviewed Jan 8, 2024

View reviewed changes

src/services/organizeImports.ts Outdated Show resolved Hide resolved

tests/cases/fourslash/autoImportTypeImport3.ts Outdated Show resolved Hide resolved

tests/cases/fourslash/importNameCodeFix_importType10.ts Outdated Show resolved Hide resolved

fixed tests and verify.importFixes

42033fc

andrewbranch approved these changes Jan 9, 2024

View reviewed changes

fix verifyImportFixAtPosition

dca9ba6

jakebailey reviewed Jan 9, 2024

View reviewed changes

jakebailey approved these changes Jan 9, 2024

View reviewed changes

tests/cases/fourslash/autoImportTypeImport4.ts Show resolved Hide resolved

iisaduan added 2 commits January 9, 2024 15:18

add comments

d408f5a

update baselines

7902488

jakebailey approved these changes Jan 10, 2024

View reviewed changes

iisaduan merged commit 7800978 into microsoft:main Jan 10, 2024
19 checks passed

iisaduan deleted the organize-type-imports branch January 10, 2024 05:38

This was referenced Sep 17, 2024

fmt with verbatimModuleSyntax? denoland/deno#22583

Closed

feat: ability to sort type only named imports and exports first or last dprint/dprint-plugin-typescript#664

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Organize type imports #55269

Organize type imports #55269

iisaduan commented Aug 4, 2023 •

edited

Loading

typescript-bot commented Aug 4, 2023

typescript-bot commented Aug 4, 2023

jakebailey Aug 4, 2023

iisaduan Aug 4, 2023

jakebailey Aug 4, 2023

jakebailey commented Aug 4, 2023

andrewbranch left a comment

andrewbranch left a comment

andrewbranch Jan 9, 2024

jakebailey left a comment

jakebailey Jan 9, 2024

iisaduan Jan 9, 2024

jakebailey left a comment

iisaduan commented Jan 9, 2024

iisaduan commented Jan 9, 2024 •

edited

Loading

andrewbranch commented Jan 9, 2024

iisaduan commented Jan 9, 2024

jakebailey commented Jan 9, 2024

Organize type imports #55269

Organize type imports #55269

Conversation

iisaduan commented Aug 4, 2023 • edited Loading

typescript-bot commented Aug 4, 2023

typescript-bot commented Aug 4, 2023

jakebailey Aug 4, 2023

Choose a reason for hiding this comment

iisaduan Aug 4, 2023

Choose a reason for hiding this comment

jakebailey Aug 4, 2023

Choose a reason for hiding this comment

jakebailey commented Aug 4, 2023

andrewbranch left a comment

Choose a reason for hiding this comment

andrewbranch left a comment

Choose a reason for hiding this comment

andrewbranch Jan 9, 2024

Choose a reason for hiding this comment

jakebailey left a comment

Choose a reason for hiding this comment

jakebailey Jan 9, 2024

Choose a reason for hiding this comment

iisaduan Jan 9, 2024

Choose a reason for hiding this comment

jakebailey left a comment

Choose a reason for hiding this comment

iisaduan commented Jan 9, 2024

iisaduan commented Jan 9, 2024 • edited Loading

andrewbranch commented Jan 9, 2024

iisaduan commented Jan 9, 2024

jakebailey commented Jan 9, 2024

iisaduan commented Aug 4, 2023 •

edited

Loading

iisaduan commented Jan 9, 2024 •

edited

Loading