Skip to content

fix(lsp): improve auto-imports to take into account only package exports#28131

Closed
dsherret wants to merge 16 commits intodenoland:mainfrom
dsherret:auto_imports_start
Closed

fix(lsp): improve auto-imports to take into account only package exports#28131
dsherret wants to merge 16 commits intodenoland:mainfrom
dsherret:auto_imports_start

Conversation

@dsherret
Copy link
Copy Markdown
Contributor

@dsherret dsherret commented Feb 15, 2025

TypeScript's auto-import implementation is implemented specifically for node_modules. Additionally, it seems to traverse through all the source files in a program then filter out any results that aren't importable. This changes things so that we instead provide TypeScript with a limited list of modules to resolve exports from (the entrypoints of every package from JSR or npm in a project) and then modify it not do a subsequent filter.

Fixes .. #28412.
Fixes #27700.

// todo: check if the referrer is cjs and provide require
ResolutionMode::Import,
NodeResolutionKind::Types,
);
Copy link
Copy Markdown
Contributor Author

@dsherret dsherret Feb 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to add some per-package caching for this because it will be slow for certain packages (because it requires traversing the directories when using wildcards)

const excludePatterns = preferences.autoImportFileExcludePatterns && getIsExcludedPatterns(preferences, useCaseSensitiveFileNames2);
forEachExternalModule(program.getTypeChecker(), program.getSourceFiles(), excludePatterns, host, (module2, file) => cb(
const importableFilePaths = host.getExportImportablePathsFromModule(referrerFile.path);
const files = importableFilePaths.map((path) => program.getSourceFile(path)).filter((s) => s != null);
Copy link
Copy Markdown
Contributor Author

@dsherret dsherret Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated why the tests are now failing and it's because the discovered files aren't in the program anymore. I think maybe we should have them in getPackageJsonAutoImportProvider (files2 below)?

@nayeemrmn
Copy link
Copy Markdown
Contributor

Update: I've found the proper fix for the quick fix import-reverse-mapping in #28412. See #29194.

I edited the description of this PR to address #27700 instead.

@dsherret dsherret closed this Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSP: Investigate /$node_modules/ hack for perf issues

2 participants