module_resolution=none or =explicit

[alexeagle]

Discussed in-person with @DanielRosenwasser

In google we have performance issues with tsc and with editor plugins. We have tracked this down to the node moduleResolutionKind looking in many locations on a network-mounted disk.

We don't actually want the node moduleResolution, we only want the baseUrl/paths/rootDirs feature known as "path mapping". Put another way, we explicitly list locations for all inputs, and we never want the compiler to stat a file outside of the explicit input locations.

For tsc, we provide a custom CompilerHost that avoids touching the disk, we implement fileExists and directoryExists purely by consulting our file manifest. However, we can't do this trick for editors, which are becoming increasingly unusable.

As an example, we provide interop with Closure Compiler code that declares goog.module, by naming these goog:some.module. This results in the language services looking for a file like goog:some.module.d.ts all over the network-mounted filesystem, even though a later sourceFile in the program has declare module 'goog:some.module'

cc @vikerman @rkirov

[rkirov]

An even simpler proposal would be for the module resolve logic to use tsconfig's 'files' listed (together with noResolve true), in determining which files exist and which do not. AFAICT it is already an error to resolve to a file that is on disk but not listed in 'files', so instead of hitting disk, TS can check against the list of absolute path listed in files.

To expand on Alex's example above, try using 'import * as fs from 'fs'" in a TS project that has 'node.d.ts' explicitly listed in 'files'. Module resolution will look for 20+ files (more depending on the directory depth), before giving up. At the end it turns out that 'declare module "fs"' is inside 'node.d.ts' and there is no file named 'fs.d.ts', or 'fs.ts'.

[mhegazy]

An even simpler proposal would be for the module resolve logic to use tsconfig's 'files' listed (together with noResolve true), in determining which files exist and which do not. AFAICT it is already an error to resolve to a file that is on disk but not listed in 'files', so instead of hitting disk, TS can check against the list of absolute path listed in files.

Module resolution has dual purpose, 1. to find what a module import points to and 2. to expand the set of input files. if --noResolve is not passed we are not expanding the set of files, but we still need to know what a module name maps to, and this is dependent on what files are on disk.

To expand on Alex's example above, try using 'import * as fs from 'fs'" in a TS project that has 'node.d.ts' explicitly listed in 'files'. Module resolution will look for 20+ files (more depending on the directory depth), before giving up. At the end it turns out that 'declare module "fs"' is inside 'node.d.ts' and there is no file named 'fs.d.ts', or 'fs.ts'.

This is a bug that we need to fix. it is tracked by #10572. if we are in a declaration file, 1. an import should never resolve to an implementation file, and 2. we should look inside the file first before looking on disk, cause this is cheaper.

For tsc, we provide a custom CompilerHost that avoids touching the disk, we implement fileExists and directoryExists purely by consulting our file manifest. However, we can't do this trick for editors, which are becoming increasingly unusable.

Is this on every edit? or on first load? how many operations are you seeing and how often are these being triggered?

[mhegazy]

@alexeagle and @rkirov can you give tonight's typescript@next a try and see if you get better experience?

[rkirov]

I just did, and indeed a number of resolutions has gone down. The modules declared inside 'node.d.ts' like 'fs', 'crypto' are picked up without extra disk lookup. Thanks!

However, there are still some extra resolutions (tied to our overlay of generated files) that make the VSCode experience janky. I would like to describe them in isolation, but I need some time to narrow it down.

One thing that confuses me is that I see a large difference between the paths looked by tsc --traceResolutions and strace attached to a tsserver.js process. Do you have a recommended way to do --traceResolutions through tsserver?

[mhegazy]

@rkirov is this still an issue?

[rkirov]

It is a bit better, but there are still a lot of file stats.
I am testing typescript@next (Version 2.2.0-dev.20161221)

I have two files in my project:
a/b/c/d/e/f/g/h/a.ts - import 'b';
typings.d.ts -

declare module 'b' { ... } 

tsc --traceResolution shows attempts to stat 150+ files before it gives up and uses the module declaration.

P.S. I realize that all the stats might be needed because if those files exist they would take precedence over the 'declare module'. What makes things worse is that it appears that vscode does this resolution many many times in a row (instead of the just once for tsc).

[evmar]

Random thought: we could reconstruct a demo of this for the TS/VSCode teams by using a module like
https://www.npmjs.com/package/sleep
to add a sleep for ~5ms in the various file access functions in ts.sys.

[FredKSchott]

Wanted to bump this thread since Snowpack TS users are also running into problems with TypeScript's reliance on Node.js module resolution logic. More info: #27481

TypeScript users on Snowpack are also running into this issue. ESM imports in Snowpack are based on the source filename, so import './file.ts' and import Counter from './counter.tsx' are both expected to work. We ask TS users to use the ".js" file extension as a workaround, but we're looking to add new features that might break this for them entirely.

It feels odd that TypeScript wants to both stay out of import resolution (the right call, imo) but also put arbitrary limits in place. I'm +1 this warning as a default to protect new users from footguns in most projects, but projects like Deno and Snowpack should be able to suppress this.

[FredKSchott]

Bump again: @RyanCavanaugh I know the team has said no to getting involved with import resolution (re: #16577) but I just wanted to make sure that this different use-case/ask didn't get wrapped into that decision incorrectly.

The idea that import "./App.ts" is invalid assumes a Node.js or browser environment. But different environments like Snowpack and Deno support importing TS files. In Snowpack's case, it's part of a larger handling of importing source files by source name, so that something like import "./App.svelte" is valid. Because Snowpack handles your build, we are able to resolve the import correctly at build time. But for TS, this behavior is broken.

It would be really useful if this warning could suppressed via an opt-in compiler option, for environments like Deno and Snowpack where importing the TS file by path is actually the correct user behavior.

This request is aligned with the decision in #16577 imo, because we are specifically not asking for any sort of resolution/rewriting of imports.

/cc @kitsonk