Expand auto-import to all dependencies in package.json

[DanielRosenwasser]

Background

TypeScript's auto-import functionality has some really unpredictable behavior today where the feature doesn't work on for a package until a user manually imports from that package. This experience only affects projects that provide their own types because, by default, TypeScript always includes @types packages as part of every compilation; however, it doesn't include other packages in node_modules^[1]. Apart from being unfair to TypeScript library authors, this is just plain surprising for users.

For a while now, we've received consistent feedback that this is a confusing experience.

• VSCode Angular 9 Auto-Import for Custom Libraries #37222
• Auto import in angular typescript is not working #28773 (comment)
• Auto Import not working with node_modules #30033
• Can't use auto import when node_modules are named "node_modules" #29629
• Auto import in angular typescript is not working #28773
• Auto-imports from builder-util-runtime #38176
• https://twitter.com/alephnaught2tog/status/1245026801564467203

Additionally, we've been seeing weird hacks in projects like Angular CLI, where typeRoots are set to node_modules and node_modules/@types to try to fix auto-imports.

Proposal

In #31893 and #32517, we improved some rough areas of completions by making sure that we only provided completions from packages that are listed in package.json. That way clearTimeout doesn't appear in your completion list unless you specifically list @types/node as a dependency.

What I'd like to suggest is an expansion of completions sources to those in package.json. Specifically what I'm looking for is that for every dependency in package.json, we should try to provide auto-imports. This could be done by changing the default behavior for typeRoots (potentially breaking) or by adding an editor-specific behavior for automatic package inclusion.

There's things to keep in mind as we do this:

• we don't want to accidentally load up 100s of .d.ts files unnecessarily
• we don't want to have duplicate entries when a package ships its own .d.ts files but a user has @types packages installed
• we don't want to add new logic that affects module resolution and changes the behavior of go-to-definition
• we don't want to drastically diverge in behavior from the command-line

Maybe for the general case, this won't cause a drastic increase in memory usage - most packages with explicit dependencies might get installed/eventually used anyway. But if it is, there might be some workarounds to consider:

• only go through 2-3 levels of imports for these expanded completions
  • this might not work well for "barrels" (i.e. files with a lot of export * from statements), but it might just be good enough.
• only expand auto-completions to dependencies and not devDependencies
  • this would be really confusing for utilities and test code, but not application code.

I'd like to get people's thoughts on whether an approach like this would even be possible, and if so, whether we could implement it without degrading the current experience.

---

^[1] The reason for this is the asumption that having an @types package in your node_modules directory is a pretty good sign that you intend to use it (with caveats - see #31893 and #32273). But in order to save on time/memory, we don't load up .d.ts files from other node_modules packages unless we really need to.

[DanielRosenwasser]

Also, thanks to @andrewbranch who explained why this behavior was happening, and who gave me the rundown of what his PRs did in #31893 and #32517 - both which were the inspiration for using the package.json-focused approach proposed here.

[andrewbranch]

First I want to link #36042, which I’ve been using as the canonical issue to reconsider this behavior. I also explained in some detail how auto-imports work today and some ideas (and corresponding pitfalls) about how we could change them to solve this.

This could be done by changing the default behavior for typeRoots (potentially breaking) or by adding an editor-specific behavior for automatic package inclusion.

I want to expound on this a little bit for the benefit of others who aren’t as familiar with what this means, and to emphasize “potentially breaking” a bit more. What you’re getting at here, which I also explained in #36042, is that today, auto-imports for a particular module are offered if and only if that module is already part of your whole-program compilation, which is determined by:

• the files covered by files/include/exclude in your tsconfig.json (called “root files”, defaulting to everything outside of node_modules with a ts/tsx extension)
• the lib files covered by lib in your tsconfig.json
• the type definition covered by types and typeRoots in your tsconfig.json (defaulting to everything in node_modules/@types)
• the recursive dependencies of all of the above (anything imported or triple-slash referenced by something already included by any of these four criteria)

Any file matched by these criteria are automatically parsed and bound, and type-checked to some degree, depending on the file location and skipLibCheck. The information produced by parsing, binding, and checking these files is central to the ability to offer auto-imports from them. At the same time, though, these criteria are also used when compiling a program with tsc on the command line. On one hand, we’ve seen in the past that automatic type inclusion, or even inclusion of additional types by recursive dependencies, creates problems of conflicting environmental globals, which has been the subject of many other discussions and proposals. I have personally advocated in the past for easing some of the pain of this problem by not eagerly including all of node_modules/@types in programs. The details of global/environment collisions aren’t super important here; rather, my point is that we know automatic type inclusion does break people, and there are potentially some merits (with tradeoffs) to doing less of it when we look at other problems. But on the other hand, doing less of it can also make TypeScript unaware of globals declared by @types packages, which is also breaking. In other words, any change to the rules that govern what files a program contains, including default values of related tsconfig options, is a significant breaking change.

A part of me thinks it would make a lot of sense to use package.json files to guide type definition inclusion, and stop giving special behavior to @types. It would solve the auto-import problem automatically, and in some cases it would prevent unwanted inclusion of definitions like @types/node that can sometimes cause problems. But another part of me thinks that’s a really hard sell for a breaking change to tsc. I think it’s worth talking about, but my assumption in thinking about this so far has been that it’s not going to be on the table.

---

In that case, the only option is, as you said, adding a language-service-specific implementation that doesn’t affect program semantics, and doesn’t touch tsc at all. So, a couple thoughts on that:

Maybe for the general case, this won't cause a drastic increase in memory usage - most packages with explicit dependencies might get installed/eventually used anyway.

I completely agree with this. If people have packages listed in their package.json, they’re going to import or reference them somehow. I’m not at all concerned about the memory impact of loading types from package.json-listed dependencies; I’m only concerned about their semantic impact on the program. So if we need a way to isolate these types from influencing the core program, how do we do that? Load them into a second program? That would take care of all the parsing, binding, and checking that auto-imports relies on, but it would duplicate a lot of work and types and symbols from common lib files (not to mention the weight of all the closed-over functions in the program/checker)—that’s the concern I have with memory. We should do some experiments and see what the cost of that really is. I also think it’s worth seeing how much we can do with parsing and binding alone. But both of these ideas are pretty complicated in an editor scenario, where the user is continually installing and uninstalling node modules and changing the shape of the core program by adding and removing imports.

---

Takeaways:

• I guess I didn’t comment on whether your proposal is the right expected behavior, because I’ve thought it’s the right expected behavior since I got here, and I always imagined Updated: Only auto-import from package.json #32517 as the first step in a process toward that goal.
• I want to hear thoughts about changing default type definition inclusion rules to look at package.json before ruling it out.
• Assuming we don’t change type definition inclusion rules, I want to hear more thoughts on an editor-specific implementation. What have I missed in my analysis? Is there a simpler approach? Do others agree with my assessment of the complexity of the approaches I’ve mentioned?

[DanielRosenwasser]

I’m not at all concerned about the memory impact of loading types from package.json-listed dependencies; I’m only concerned about their semantic impact on the program. So if we need a way to isolate these types from influencing the core program

One of the things that @ahejlsberg mentioned in a conversation was potentially using a second TypeChecker instance for auto-imports and the like.

[uniqueiniquity]

I'll need to read through this tomorrow for any lucid feedback, but is there anything we can learn from Roslyn's version of this for C#? I think theirs even recommends things from packages you don't yet have installed, so I imagine they've put work into the appropriate heuristics for this, albeit tuned to their user base.

[sheetalkamat]

Assuming we don’t change type definition inclusion rules, I want to hear more thoughts on an editor-specific implementation. What have I missed in my analysis? Is there a simpler approach? Do others agree with my assessment of the complexity of the approaches I’ve mentioned?

Having different files in program from what they are in tsc is bad idea as that results in discrepancies or errors/non errors and that gets confusing. I think the idea of different program to support the auto import seems better one..

[andrewbranch]

Yeah, I agree. I haven’t seriously considered that as an option. Just to make sure it’s clear, when I say

I want to hear thoughts about changing default type definition inclusion rules to look at package.json before ruling it out.

I’m referring to changing the behavior both for tsc and for the language service (which is why it would be such a significant breaking change).

[DanielRosenwasser]

@uniqueiniquity

is there anything we can learn from Roslyn's version of this for C#

Really glad you brought this up - I briefly spoke with @CyrusNajmabadi to understand more of what Roslyn does.

I think theirs even recommends things from packages you don't yet have installed

As far as I understand, that's a quick fix, not code completion functionality. We provide an Install '@types/whatever'. quick fix, but not one for installing missing packages. If you're interested in that, it's tracked by #20649.

I also think that auto-import completions could be a bit too noisy for npm packages. There's over a million packages on npm, and while Roslyn seems to rely on an index of the most popular NuGet packages, it's partially out of date and we might have a harder time distinguishing between common identifiers, especially functions named assert or it or describe from different testing frameworks. Maybe this would be fine as a quick fix, though we already have a few of these for identifiers like $, Buffer, etc. (details here).

This all did make me wonder: instead of digging through .d.ts files and creating a separate TypeChecker or Program, whether there's some potential to create a digest of identifiers ahead of time; something to know which exports exist without resolving/scanning/parsing/binding each package's .d.ts files. Maybe LSIF could be useful? But the question is when it's generated and how to get it.

[uniqueiniquity]

As far as I understand, that's a quick fix, not code completion functionality. We provide an Install '@types/whatever'. quick fix, but not one for installing missing packages. If you're interested in that, it's tracked by #20649.

Not necessarily interested; my point was just that it seemed like a good sign they had substantially thought through this. :)

Maybe LSIF could be useful? But the question is when it's generated and how to get it.

I think LSIF could definitely be useful for this - in fact, this is kinda what it's for (see lsif-npm). I'm not sure where this information is currently hosted or how often it's generated, though.