diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 5124b47f47..7bf057a03d 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -743,11 +743,7 @@ namespace ts.codefix { const symbolIdActionMap = new ImportCodeActionMap(); const currentTokenMeaning = getMeaningFromLocation(symbolToken); - forEachExternalModule(checker, allSourceFiles, moduleSymbol => { - if (moduleSymbol === sourceFile.symbol) { - return; - } - + forEachExternalModuleToImportFrom(checker, sourceFile, allSourceFiles, moduleSymbol => { cancellationToken.throwIfCancellationRequested(); // check the default export const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); @@ -776,17 +772,35 @@ namespace ts.codefix { return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)); } - export function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray, cb: (module: Symbol) => void) { + export function forEachExternalModuleToImportFrom(checker: TypeChecker, from: SourceFile, allSourceFiles: ReadonlyArray, cb: (module: Symbol) => void) { + forEachExternalModule(checker, allSourceFiles, (module, sourceFile) => { + if (sourceFile === undefined || sourceFile !== from && isImportablePath(from.fileName, sourceFile.fileName)) { + cb(module); + } + }); + } + + export function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray, cb: (module: Symbol, sourceFile: SourceFile | undefined) => void) { for (const ambient of checker.getAmbientModules()) { - cb(ambient); + cb(ambient, /*sourceFile*/ undefined); } for (const sourceFile of allSourceFiles) { if (isExternalOrCommonJsModule(sourceFile)) { - cb(sourceFile.symbol); + cb(sourceFile.symbol, sourceFile); } } } + /** + * Don't include something from a `node_modules` that isn't actually reachable by a global import. + * A relative import to node_modules is usually a bad idea. + */ + function isImportablePath(fromPath: string, toPath: string): boolean { + // If it's in a `node_modules` but is not reachable from here via a global import, don't bother. + const toNodeModules = forEachAncestorDirectory(toPath, ancestor => getBaseFileName(ancestor) === "node_modules" ? ancestor : undefined); + return toNodeModules === undefined || startsWith(fromPath, getDirectoryPath(toNodeModules)); + } + export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget): string { return moduleSpecifierToValidIdentifier(removeFileExtension(getBaseFileName(moduleSymbol.name)), target); } diff --git a/src/services/completions.ts b/src/services/completions.ts index 083ddea37d..2e9a26c688 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1031,11 +1031,7 @@ namespace ts.Completions { function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void { const tokenTextLowerCase = tokenText.toLowerCase(); - codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => { - if (moduleSymbol === sourceFile.symbol) { - return; - } - + codefix.forEachExternalModuleToImportFrom(typeChecker, sourceFile, allSourceFiles, moduleSymbol => { for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) { let { name } = symbol; diff --git a/tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts b/tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts new file mode 100644 index 0000000000..44392f8b87 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_notFromUnrelatedNodeModules.ts @@ -0,0 +1,10 @@ +/// + +// @Filename: /unrelated/node_modules/@types/foo/index.d.ts +////export function foo() {} + +// @Filename: /src/b.ts +////fo/**/; + +goTo.marker(""); +verify.not.completionListContains({ name: "foo", source: "/unrelated/node_modules/@types/foo/index" }, undefined, undefined, undefined, undefined, undefined, { includeExternalModuleExports: true }); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts index 2410036299..fa483a8dde 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportNodeModules5.ts @@ -18,8 +18,5 @@ //// "types": "bin/lib/libfile.d.ts" //// } -verify.importFixAtPosition([ -`import { f1 } from "package-name/node_modules/package-name2"; - -f1('');` -]); +// No fix because this accesses a nested node_modules +verify.not.codeFixAvailable();