Skip to content

"Organize imports" removes imports necessary for declaration file #22974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dsherret opened this issue Mar 29, 2018 · 2 comments
Closed

"Organize imports" removes imports necessary for declaration file #22974

dsherret opened this issue Mar 29, 2018 · 2 comments
Labels
Duplicate An existing issue was already created

Comments

@dsherret
Copy link
Contributor

dsherret commented Mar 29, 2018

TypeScript Version: [email protected]

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
Copy link
Contributor

mhegazy commented Mar 29, 2018

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.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Mar 29, 2018
@dsherret
Copy link
Contributor Author

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!

@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

2 participants