Don’t show auto-import suggestion if there’s a global by the same name by andrewbranch · Pull Request #31065 · microsoft/TypeScript (original) (raw)

Fixes #30713

Locals already shadow auto-import suggestions, but people were having issues with globals like clearTimeout being auto-imported from 'timers', for example.

I'm not 100% sure this is the right thing to do. Thinking about the clearTimeout example:

// node_modules/@types/node/globals.d.ts declare function clearTimeout(timeoutId: NodeJS.Timeout): void;

// node_modules/@types/node/timers.d.ts declare module 'timers' { function clearTimeout(timeoutId: NodeJS.Timeout): void; }

(Depending on the user’s tsconfig, they’ll likely have another ambient declaration in lib.dom.d.ts, but it becomes an overload with @types/node’s ambient declaration, so it’s not particularly relevant to the problem.)

Between these two declarations, I don't care about the one in timers at all. I never ever want to import clearTimeout from timers. Why?

1. If I'm writing for node, I don't want the import because it's equivalent to the global, and it's unconventional to import it. I probably don't even know that a module called “timers” exists.
2. If I'm writing for the browser, I probably have @types/node on accident or indirectly, and the import actually won’t work because I'm writing for the browser.

On the other hand, I can envision some scenarios where I have different motivations and desired outcomes:

// file.ts export class File {}

// index.ts const file = new Fi/**/

There's a DOM global File, but as files are a pretty common thing to talk about in computing, it's also quite common to write a type or class called File in your own project. In this case, I probably want to auto-import my own File class. The global gets ranked higher, but at least I can press the down arrow key and get the auto-import. If we merge this PR, nothing called File will ever be auto-importable (in a program that has lib.dom.d.ts).

Observations:

• We can't actually know that the two clearTimeout declarations in @types/node are truly the same runtime function, but it seems highly likely since they’re both in @types/node.
• If a module export shares the same name with a global, the chances that you want to import that thing instead of use the global are probably much higher for a local module like file.ts than for a module we found in node_modules like timers.
• @types/node is special, because it’s the only exception I can think of to the rule: “If I have a package.json, I do not want to auto-import from any package not listed in it.”

Possible solutions:

• Retain same-name auto-import suggestions for local modules, suppress them for node_modules
• Suppress auto-import suggestions that have the same name as an ambient declaration from the same package; i.e. clearTimeout from timers gets hidden because it’s in @types/node and so is a global one.
• Just go with this PR and see if compelling arguments for doing it another way arise in practice.