"Organize imports" removes imports necessary for declaration file

[dsherret]

TypeScript Version: typescript@2.9.0-dev.20180328

Search Terms: organize imports declaration

Code

Take this file:

import {NoteStore} from "./NoteStore";
import {Note} from "./Note";

export class SomeClass {
    constructor(private readonly noteStore: NoteStore) {
    }

    getNotes() {
        // implicit return type: Note[]
        return this.noteStore.getValues();
    }
}

With compiler option declaration: true

Expected behavior:

import {Note} from "./Note";
import {NoteStore} from "./NoteStore";

// ...class declaration omitted...

Actual behavior:

import {NoteStore} from "./NoteStore";

// ...class declaration omitted...

And causes this compile error: Return type of public method from exported class has or is using name 'Note' from external module "fileNameGoesHere.ts" but cannot be named.

Playground Link: N/A

By the way, I ran "organize imports" on about 800 files. Works really well other than this problem and it puts imports that were previously on multiple lines onto one... so that will break my linting rule for "max-line-length". Luckily it's easy to write a script to investigate and break them back onto multiple lines 😄

[mhegazy]

For background, the issue is the declaration emitter will not add an import to a module that was not originally imported. that is why it requires the user to import a module if it is needed to write the type of a declaration in the current file. For more information see #9944.

The correct fix in these cases is two parts, 1. import the modules, and 2. use the imported module to add an explicit type annotation on the declaration in question.. this avoids errors with --noUnusedLocals for instance (see #13694).

We have an issue to automate this process using a quick fix, see #22132.

The correct fix here is to address #9944. i.e. make the declaration emitter write the import it needs and not error.

There is one other issue referenced in the OP that we should capture as well, filed #22991 to track it.

[dsherret]

The correct fix here is to address #9944. i.e. make the declaration emitter write the import it needs and not error.

Completely agree. Thanks @mhegazy!