Reprioritize cross-project module specifier suggestions for auto-import

[andrewbranch]

Supersedes #39267.
Fixes #38856.

Best way to understand this PR:

1. Read this description. Don’t worry if it’s opaque.
2. Read the fourslash tests, ignoring autoImportCrossProject_paths_sharedOutDir for now. Particularly, note there is one group of tests that simulate lerna-style symlinks, and one group that relies only on paths, which simulates a bundler/post-build step that transforms module specifiers. Note that the verify assertions in each group show that I can tweak paths to make auto-import give me whichever module specifier I want, choosing between "dep/src/sub/folder", "dep/dist/sub/folder", and "dep/sub/folder".
3. Read autoImportCrossProject_paths_sharedOutDir if desired—it was a representation of a real-world project structure from a user with a slightly different setup, but it’s functionally redundant.
4. Re-read this description if it wasn’t clear initially.
5. Review the implementation, primarily surrounding the comments in getModuleSpecifiers.

Background

• Historically, we have generated module specifiers by considering only input (TypeScript) files. The assumption is that the relative path from TS file to TS file is identical to the relative path between the corresponding output JS files, even if those files are not colocated with their inputs (i.e., they are placed in an outDir). This always holds for files in a single project. However, it may not hold for paths from an input file in one project to an input file in another composite project. Whether it holds is dependent upon the project settings of each project. As a rule, TypeScript does not care whether it holds. This is because the actual runtime organization of composite projects may not be similar to their organization on disk during development. For example, a user might package each composite project individually and publish each to a package registry. Post-compilation build steps and deployment strategies are outside TypeScript’s scope of concern.
• In Handle auto import scenarios when using project references #37482, we added an exception to the rule of generating module specifiers by considering only paths from input to input. When the imported file is from another project, we started considering paths from input (TS) to output (JS/.d.ts). The motivation for this change was to recognize when a path like ../node_modules/symlinked-project/src/index.ts has a corresponding output like ../node_modules/symlinked-project/dist/index.d.ts, such that if ../node_modules/symlinked-project/package.json includes "types": "dist/index.d.ts", we could generate the bare package specifier "symlinked-project".

Problem

Generating a bare package specifier like "symlinked-project" instead of "symlinked-project/src", when possible, is almost always the specifier the user prefers. The problem exhibited in #38856 is the effect of #37482 when the output file is not the file specified by the "types" field of the package’s package.json (or when the path to the output file did not go through node_modules at all). In these cases, we generate a path that explicitly maps from input (TS) to output (.d.ts, typically colocated with JS). Moreover, this path from input to output is the only module specifier we would suggest. My first PR addressing this problem, #39267, claimed that such a path was always undesirable. However, that’s not the case. You can trivially imagine publishing/deployment strategies where, at runtime, the correct specifier is "some-dependency/dist/foo". Such a path is quite possibly more likely to be correct than the alternative "some-dependency/src/foo", but it depends on variables outside TypeScript’s scope of concern.

Further, the desired specifier may be "some-dependency/foo", if some-dependency is published from its dist directory. When some-dependency is a composite project symlinked through node_modules (as with lerna or yarn workspaces), TypeScript could be configured to resolve some-dependency/foo correctly by setting up paths, but auto-import would never suggest some-dependency/foo, because when a possible path through node_modules is found, paths are not considered.

Solution

1. To bring us back in line with historical precedent, reject paths from input to output unless they result in a bare package specifier (e.g. "symlinked-dependency") by merit of the output file matching its package.json’s "types" field.
2. When a bare package specifier cannot be suggested, ensure that paths can always be used to customize the preferred specifier to a file in another project, even when a path through node_modules (e.g. "symlinked-dependency/src/foo") is available.

Point (1) preserves the original intent of #37482, while eliminating paths that explicitly point to a composite project’s outDir by default (fixing #38856). Point (2) covers, I believe, all ambiguity between "symlinked-dependency/src/foo", "symlinked-dependency/dist/foo", and "symlinked-dependency/foo", which ought to account for any objections that fixing #38856 may actually be wrong for some users.

[bradennapier]

This sounds great! Thanks for the hard work on this issue!!

[dko-slapdash]

@andrewbranch Sorry for the noice I made yesterday in different issues/PRs, I wasn't sure which one is more relevant and made a number of communication mistakes. Impressed how responsive you folks are: tracking every single comment in all issues of such a huge project is unbelievably cool.

Regarding the change in this particular PR - it sounds like the answer to the original question. I just have several clarification questions if you don't mind. I think that the related topic is not yet covered much in the TS documentation pages.

1. In all the unit tests, baseUrl=".", and it looks like it's an essential requirement to have it "." to make the symlink-to-dist mapping work (at least I couldn't make baseUrl work if it's not ".'). But what if I need baseUrl to be "src" (i.e. to be able to include the same-project files using absolute paths not including src/ in it)? I was actually able to "work-around" it with one extra paths mapping "*" => "src/*" added to the very end of paths; wondering whether it is a correct way to achieve this?
2. We map "shared/dist/*" to "../shared/src/*" - i.e. dist/ to src/. And at the same time, we have references directive which, according to the docs, saves compilation time and always prefers inclusion of *.d.ts files for performance reasons. The question is, will this dist/->src/ mapping break this perf optimization, or TS will still magically prefer *.d.ts files with it (which is desired)?

[andrewbranch]

No worries @dko-slapdash, I just wanted to have a single place to continue the discussion.

1. Note that the relative paths in the values (not keys) of paths are realtive to baseUrl, if baseUrl is set. Could that be why you had trouble with setting it to src? I think the extra path mapping you added is ok, but I’m pretty sure it should also work with baseUrl set to a subdirectory. Let me know if that’s not the case.
2. The magic still works. Regardless of whether module resolution ends at an input or output of a referenced project, the compiler knows it’s part of a referenced project and knows how to map those inputs to outputs and vice versa, so it always uses the one that it should for the given scenario (declarations for tsc -b, sources for TS Server, unless settings direct otherwise).

I’m also really curious about your runtime setup—this PR was just one part of a broader effort I’ve been working on to make TypeScript work better for monorepos with all different kinds of infrastructure. You mentioned

i.e. to be able to include the same-project files using absolute paths not including src/ in it

How do these resolve at runtime? Are you using Webpack or another bundler? A custom module loader in Node? Is the project you’re working on an open source repo that I could take a look at? It would be great to add this sort of thing to my list of real-world monorepo setups so I can consider it in future work.

[dko-slapdash]

Thanks for your reply!

A) I tried baseUrl=src and all sorts of ../.. in paths, but it didn't work, IntelliSense still proposes src and not dist. I'll try again and post a screenshot.
B) We use typescript-transform-paths plugin: https://www.npmjs.com/package/typescript-transform-paths (v1.x since v2 is not fully working for us yet, conflicts with other plugins). It converts all paths to relative automatically in both build and watch mode, so in dist/, all paths appear to be relative. Ryan told me that there are zero chances that such a path mapping would ever be built in to TS itself, but there is a plugin (based on ttypescript library).

[dko-slapdash]

Yeah, basically, with baseUrl=src and paths equal to shared/dist/* => ../../shared/src/* it works exactly as expected.

There is miscompatibility in this setting with typescript-transform-paths though (because it also picks this paths change and produces the unexpected ../../... output), but it's a different topic.

[dko-slapdash]

Related to the topic (the info for future readers): here is #40101 which allows to use the above technique even without having baseUrl defined (if so, paths array values are related to the current config's directory). Which is cool.

Motivation: we switched to within-module-all-relative paths (../../..), and we wanted to have no baseUrl defined anywhere at all. Because it's safer: without baseUrl, VSCode won't even propose non-relative imports in auto-import IntelliSense feature, it would always propose ../.. paths, and there would be no risk of leaking "wrong" paths to generated dist/*.js files. (In the past we used typescript-transform-paths, but now we moved off it to reduce complexity.)

Here is the final workable layout. It also illustrates the case when folders in packages/* have different names than package names in package.json files ("my-" prefix) - not something to be proud of though, but in case it's needed, it works too.