Autocomplete incorrectly suggests transitive files from monorepo package

[ericnorris]

🔎 Search Terms

"autocomplete transitive", "autocomplete export null", "auto-import workspace package", "monorepo auto-import"

🕗 Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about "Auto-import Heuristics and Preferences". Notably, I think this line from the FAQ is appropriate:

It operates under a few key assumptions:
...
2. It's a bad idea to import things that won't work

⏯ Playground Link

This involves several files, so I've included a fourslash test below.

💻 Code

Below is a failing fourslash test that I put at tests/cases/fourslash/server/autoImportProviderTransitiveLeak.ts:

/// <reference path="../fourslash.ts" />

// @Filename: /home/src/workspaces/project/tsconfig.base.json
//// {
////   "compilerOptions": {
////     "module": "nodenext",
////     "composite": true,
////     "outDir": "${configDir}/dist"
////   }
//// }

// packages/foo

// @Filename: /home/src/workspaces/project/packages/foo/package.json
//// { "name": "@packages/foo", "type": "module", "version": "1.0.0", "exports": { ".": { "types": "./src/index.ts", "default": "./dist/index.js" }, "./internal/*": null } }

// @Filename: /home/src/workspaces/project/packages/foo/tsconfig.json
//// { "extends": "../../tsconfig.base.json" }

// @Filename: /home/src/workspaces/project/packages/foo/src/internal/index.ts
//// export function fooInternal() { console.log("foo"); }

// @Filename: /home/src/workspaces/project/packages/foo/src/index.ts
//// import { fooInternal } from "./internal/index.js"
////
//// export function foo() { fooInternal(); }

// packages/bar

// @Filename: /home/src/workspaces/project/packages/bar/package.json
//// { "name": "@packages/bar", "type": "module", "version": "1.0.0", "exports": { ".": { "types": "./src/index.ts", "default": "./dist/index.js" }, "./internal/*": null }, "dependencies": { "@packages/foo": "*" } }

// @Filename: /home/src/workspaces/project/packages/bar/tsconfig.json
//// { "extends": "../../tsconfig.base.json" }

// @Filename: /home/src/workspaces/project/packages/bar/src/index.ts
//// fo/**/

// npm workspaces

// @Filename: /home/src/workspaces/project/package.json
//// { "workspaces": ["packages/*"], "type": "module" }

// @link: /home/src/workspaces/project/packages/foo -> /home/src/workspaces/project/node_modules/@packages/foo
// @link: /home/src/workspaces/project/packages/bar -> /home/src/workspaces/project/node_modules/@packages/bar

goTo.marker("");

verify.completions({
  marker: "",
  includes: [
    {
      name: "foo",
      source: "@packages/foo",
      sourceDisplay: "@packages/foo",
      hasAction: true,
      sortText: completion.SortText.AutoImportSuggestions,
    },
  ],
  excludes: ["fooInternal"],
  preferences: {
    includeCompletionsForModuleExports: true,
    allowIncompleteCompletions: true,
  },
});

🙁 Actual behavior

When replicating this in a repository, autocomplete suggests fooInternal via a relative path import. Accepting this import results in the following code that has a red squiggly on the import line and fails to compile:

import { fooInternal } from "../../foo/src/internal/index.js";

fooInternal

The error is:

File '<elided>/packages/foo/src/internal/index.ts' is not under 'rootDir' '<elided>/packages/bar/src'. 'rootDir' is expected to contain all source files.

🙂 Expected behavior

I believe that fooInternal should not be suggested because it's "a bad idea to import things that won't work" per the FAQ. This import won't work for two reasons:

1. It is not in the current Typescript project, nor is it referenced.
2. Even if it suggested the package path and not a relative path, it is blocked by the NodeJS exports.

Additional information about the issue

After doing some initial investigation with Claude to identify the source of the bug, I noticed that getNodeModulesPackageNameFromFileName might have theoretically worked here, but it does not because these are symlinks due to being workspace packages, which are resolved to a non-node_modules directory. I tried the test with "preserveSymlinks": true, however, and the bug still occurred. This appears to be because AutoImportProviderProject unconditionally resolves the symlink, which may or may not be appropriate, I'm not sure.

I'm happy to provide more context about why we want fooInternal to not be suggested, but I hope it's clear from the reproduction case.

[RyanCavanaugh]

Does this repro in TS 7? Auto-import has been basically rewritten from scratch in that codebase

[ericnorris]

Thanks for the quick response @RyanCavanaugh! I had considered that and tried it at one point recently, but I saw the same behavior and some double autocomplete entries that I was going to leave for a separate investigation. That said, I'm trying it again now and I'm seeing almost the correct behavior - it's not suggesting the internal method, but it's also not suggesting a new method until I've compiled it, which should (?) not be necessary because types is pointing at the ts file.

I will spend some time double-checking my test setup for the new compiler however, so take this as a tentative answer for now.

[RyanCavanaugh]

Sure. FWIW we will not be fixing TS 6 LS bugs, so only testing on 7 is relevant in terms of actionable work items

[ericnorris]

I believe that the bug, as filed, is not happening in TS 7 as of this moment. With some effort I replicated the fourslash test and it passed in the typescript-go repository. It's a little hard to search for an existing test to see if this is covered, but if it isn't, I think it'd be a valuable test in order to prevent a regression!

I was able to replicate the fact that TS 7 isn't suggesting a new method until it's been compiled with a fourslash test, but I'm not sure if that's worth filing, and is separate from this GitHub issue. If you are interested, you can replicate it with the following diff (though this would deserve its own test, if you agree it is in fact a bug):

--- a/internal/fourslash/tests/autoImportSymlinkedMonorepoGranularUpdate_test.go
+++ b/internal/fourslash/tests/autoImportSymlinkedMonorepoGranularUpdate_test.go
@@ -40,7 +40,7 @@ func TestAutoImportSymlinkedMonorepoGranularUpdate(t *testing.T) {
   "version": "1.0.0",
   "exports": {
     ".": {
-      "types": "./dist/index.d.ts",
+      "types": "./src/index.ts",
       "default": "./dist/index.js"
     }
   }
@@ -59,7 +59,7 @@ export declare const projectBValue: number;
     "rootDir": "./src"
   },
   "include": ["src"],
-  "references": [{ "path": "../project-b" }]
+  "references": []
 }

My justification is that this does work in TS 6, but with the drawback of this very GitHub issue. Let me know if you think this is worth creating a separate issue in the typescript-go repository.