Auto import adds types to existing value imports under --importsNotUsedAsValues=error

[itsMapleLeaf]

TypeScript Version: 4.0.0-dev.20200705

Search Terms: auto import type

Code

// tsconfig.json
{
	"compilerOptions": {
		"target": "es2019",
		"module": "esnext",
		"noEmit": true,
		"allowJs": true,
		"importsNotUsedAsValues": "error",
		"strict": true,
		"moduleResolution": "node",
		"esModuleInterop": true,
		"forceConsistentCasingInFileNames": true,
		"maxNodeModuleJsDepth": 1
	},
	"include": ["src", "types"]
}

// secret.ts
export type Secret = number
export const secret: Secret = 42

// main.ts
import { secret } from './secret'

// type `: Secret` after `newSecret`, then tab to auto-import it
// the import is added with the existing import,
// instead of as a type-only on a new line
const newSecret = secret + 42

Expected behavior:
When auto-importing a type in a type-only place, with "importsNotUsedAsValues": "error", it should always add a type import

Actual behavior:
If there is an existing import for where the type is coming from, it'll add it to that import, instead of creating a new type-only import

Here's a video of the bug:

Playground Link: Not applicable

Related Issues: Didn't see any

[andrewbranch]

This is expected behavior. A type-only import is unnecessary if there is already a regular import from the same module. The point of type-only imports is to eliminate runtime dependencies between modules. You already have an unavoidable runtime dependency on that module, so adding a type-only import has no effect on the dependency graph. Auto-imports will only give you a type-only import if a regular import would cause an error under --importsNotUsedAsValues=error. The error, if you had one, would be telling you that you’re creating a runtime dependency you don’t need. Since you do need the runtime dependency, there’s no error (#36959), so auto-imports proceeds with its normal behavior.

[typescript-bot]

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[PinkaminaDianePie]

@andrewbranch
This is not expected behavior. Imagine situation:
I have

import { value } from 'module';

then I auto-import some types:

import { value, Type1, Type2 } from 'module';

and then some other developer will refactor this module so value won't be needed anymore:

import { Type1, Type2 } from 'module';

That could be easily replaced by

import type { Type1, Type2 } from 'module';

So if I use only auto-imports, at some point I can have unnecessary runtime dependencies only due to this bug. If it works properly, it will add only type imports, so in case if any values will be removed from the module we will have no runtime dependency. In the way it works right now, it helps to get rid of dependencies only in some cases, in other users would need to manually go through all the dependencies and check them manually and it kills the purpose of that flag. Please reopen the issue

[andrewbranch]

and then some other developer will refactor this module so value won't be needed anymore:

import { Type1, Type2 } from 'module';

^ this is an error with --importsNotUsedAsValues=error

So if I use only auto-imports, at some point I can have unnecessary runtime dependencies only due to this bug.

No; --importsNotUsedAsValues=error will prevent that code with unnecessary runtime dependencies of this form from compiling. (Also, it’s not a bug.)

in other users would need to manually go through all the dependencies and check them manually and it kills the purpose of that flag

No; --importsNotUsedAsValues=error will highlight all the problems, and a codefix is available to fix them automatically. No manual checking is necessary. No manual code editing is even necessary.

I would also be remiss not to mention the fact that import { Type1, Type2 } from 'module' does not create a runtime dependency upon module under default compilation settings. By default, import declarations that do not include any imports that are used in a value position are removed entirely. If your goal is to avoid unnecessary runtime dependencies between modules, you should generally not use the importsNotUsedAsValues setting and you should generally not use type-only imports. Your goal is already accomplished for free, by default, with no effort required of you. The purpose of the flag is to allow people to opt into runtime dependencies that would otherwise have been removed.

[PinkaminaDianePie]

My bad, I actually had import { Value, Type2 } from 'module'; instead of import { Type1, Type2 } from 'module';, so TS didn't complain about that. Anyway, I spent some time investigating it, while if we had type imports by default everywhere it will be explicit so there will be no need to investigate at all. Currently, I still have to scroll to the top of the module after each time import and move my type from the line with value imports to the line with type imports. We have an option to explicitly import types, but we have no option to auto-import types, so this process is still manual

[andrewbranch]

Currently, I still have to scroll to the top of the module after each time import and move my type from the line with value imports to the line with type imports.

I strongly disagree with this assessment. You don’t have to move your import specifiers around. There is no reason to do that other than aesthetics. My personal coding philosophy is that focusing on code aesthetic is a job for machines, not humans. TypeScript’s job is to add another layer of meaning to your code, and how you collate your import specifiers, in the absence of an error issued by --importsNotUsedAsValues=error, is meaningless. Automating this aesthetic preference could be done with an ESLint rule. Without such a rule, I would personally recommend against manual intervention. But however you choose to approach your code, an option like that is outside of TypeScript’s domain of concern, since it is semantically void.

[PinkaminaDianePie]

There are some reasons to move types to type imports.

1. Explicitness is a more important thing than just aesthetics. People need to see if something was imported as a type or as a value. I would never guess if a class was used as a type or as a value from the import if I have no clear distinction between. I have to dig into the code, I can just tell it from the import. I can't tell how many runtime dependencies the particular module has - I have to dig into the code. Hiding such things it's not adding the meaning. It's obfuscating.
2. Can you be 100% sure that type in normal import will be safely removed and make no runtime dependency? What if use babel instead of TS compiler? What if use the SWC compiler? Deno? Anything else that will be created at any point in time? We don't want to speculate and rely on details of implementation, we just want to keep types separated from values, so it will just work everywhere.

[andrewbranch]

I can't tell how many runtime dependencies the particular module has - I have to dig into the code. Hiding such things it's not adding the meaning. It's obfuscating.

I don’t really understand why “number of module dependencies” is relevant, as the total runtime size of the dependency graph from a single module could be anywhere from zero bytes to as much memory as the runtime can allocate to code compilation. There are other tools that let you inspect the size of that graph, which seems infinitely more relevant. But again, I have to emphasize that with --importsNotUsedAsValues=error, you cannot affect the set of module dependencies by separating or combining import specifiers. The set of modules that are depended upon at runtime is fixed for a given code state. If the state changes such that something is no longer used as a value, you get an error. If you really want to know how many modules you “really” depend on, put all your type-only imports together and all your regular imports together, and then just count the number of lines taken up by regular imports.

What if use babel instead of TS compiler? What if use the SWC compiler? Deno? Anything else that will be created at any point in time?

Babel does this, yes. Deno uses the TypeScript compiler. If this is a limitation of future tools, it sounds like something those tools should address. If there is a lot of demand for enforcement of this kind of rule in the future, stemming from reasonable causes other than aesthetics, it’s something we could consider. But you’re only the second person to ask for something like this for the purposes of external tools, and it sounds like your concern is still hypothetical at this point. If we were to add it now, I predict it would mostly serve to fuel meaningless stylistic dogmatism, making everyone less productive.

[dummdidumm]

Over at Svelte we have the need to strictly separate type and value imports. The use case is preprocessing the script contents of a Svelte file. Given this input:

<script lang="ts">
   import { value, valueOnlyUsedInTemplate } from './somewhere';
   import type { aType } from './somewhere';

   const foo: aType = value;
</script>
{foo} {valueOnlyUsedInTemplate}

The preprocessor invokes ts.transpileModule with only the contents of the script tag, which would be

   import { value, valueOnlyUsedInTemplate } from './somewhere';
   import type { aType } from './somewhere';

   const foo: aType = value;

Given the current mechanics of TS transpilation, both valueOnlyUsedInTemplate and aType would be removed because they are not used as a value. In case of valueOnlyUsedInTemplate that's wrong. To work around this, the TS Svelte preprocessor adds a transformation which ensures every value imported through import is preserved - but this is also true for interface imports. As a consequence the developer has to tediously separate type and value imports, because TS will merge type and value imports as soon as one value import is given for the same import location. It therefore would be great to reconsider this feature request which would mean

• a new variant of importsNotUsedAsValues with strict separation of type and value imports
• corresponding TS error if type and value import are mixed
• corresponding autocompletion intellisense which keeps imports separate and switches an import from type to value if the semantics change

This is probably what the OP wanted, which would be enabled by #43393