diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index d28e34a2bd7db..c740f8996ee80 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -789,70 +789,6 @@ namespace ts.moduleSpecifiers { } } - interface NodeModulePathParts { - readonly topLevelNodeModulesIndex: number; - readonly topLevelPackageNameIndex: number; - readonly packageRootIndex: number; - readonly fileNameIndex: number; - } - function getNodeModulePathParts(fullPath: string): NodeModulePathParts | undefined { - // If fullPath can't be valid module file within node_modules, returns undefined. - // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js - // Returns indices: ^ ^ ^ ^ - - let topLevelNodeModulesIndex = 0; - let topLevelPackageNameIndex = 0; - let packageRootIndex = 0; - let fileNameIndex = 0; - - const enum States { - BeforeNodeModules, - NodeModules, - Scope, - PackageContent - } - - let partStart = 0; - let partEnd = 0; - let state = States.BeforeNodeModules; - - while (partEnd >= 0) { - partStart = partEnd; - partEnd = fullPath.indexOf("/", partStart + 1); - switch (state) { - case States.BeforeNodeModules: - if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) { - topLevelNodeModulesIndex = partStart; - topLevelPackageNameIndex = partEnd; - state = States.NodeModules; - } - break; - case States.NodeModules: - case States.Scope: - if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") { - state = States.Scope; - } - else { - packageRootIndex = partEnd; - state = States.PackageContent; - } - break; - case States.PackageContent: - if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) { - state = States.NodeModules; - } - else { - state = States.PackageContent; - } - break; - } - } - - fileNameIndex = partStart; - - return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; - } - function getPathRelativeToRootDirs(path: string, rootDirs: readonly string[], getCanonicalFileName: GetCanonicalFileName): string | undefined { return firstDefined(rootDirs, rootDir => { const relativePath = getRelativePathIfInDirectory(path, rootDir, getCanonicalFileName); diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index e20193dc59a4a..ab7d0e6375ea5 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -7501,4 +7501,68 @@ namespace ts { export function isThisTypeParameter(type: Type): boolean { return !!(type.flags & TypeFlags.TypeParameter && (type as TypeParameter).isThisType); } + + export interface NodeModulePathParts { + readonly topLevelNodeModulesIndex: number; + readonly topLevelPackageNameIndex: number; + readonly packageRootIndex: number; + readonly fileNameIndex: number; + } + export function getNodeModulePathParts(fullPath: string): NodeModulePathParts | undefined { + // If fullPath can't be valid module file within node_modules, returns undefined. + // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js + // Returns indices: ^ ^ ^ ^ + + let topLevelNodeModulesIndex = 0; + let topLevelPackageNameIndex = 0; + let packageRootIndex = 0; + let fileNameIndex = 0; + + const enum States { + BeforeNodeModules, + NodeModules, + Scope, + PackageContent + } + + let partStart = 0; + let partEnd = 0; + let state = States.BeforeNodeModules; + + while (partEnd >= 0) { + partStart = partEnd; + partEnd = fullPath.indexOf("/", partStart + 1); + switch (state) { + case States.BeforeNodeModules: + if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) { + topLevelNodeModulesIndex = partStart; + topLevelPackageNameIndex = partEnd; + state = States.NodeModules; + } + break; + case States.NodeModules: + case States.Scope: + if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") { + state = States.Scope; + } + else { + packageRootIndex = partEnd; + state = States.PackageContent; + } + break; + case States.PackageContent: + if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) { + state = States.NodeModules; + } + else { + state = States.PackageContent; + } + break; + } + } + + fileNameIndex = partStart; + + return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; + } } diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts index ab5de5931de6f..a234e7376fa7d 100644 --- a/src/services/exportInfoMap.ts +++ b/src/services/exportInfoMap.ts @@ -32,6 +32,7 @@ namespace ts { symbolTableKey: __String; moduleName: string; moduleFile: SourceFile | undefined; + packageName: string | undefined; // SymbolExportInfo, but optional symbols readonly symbol: Symbol | undefined; @@ -63,6 +64,17 @@ namespace ts { let exportInfoId = 1; const exportInfo = createMultiMap(); const symbols = new Map(); + /** + * Key: node_modules package name (no @types). + * Value: path to deepest node_modules folder seen that is + * both visible to `usableByFileName` and contains the package. + * + * Later, we can see if a given SymbolExportInfo is shadowed by + * a another installation of the same package in a deeper + * node_modules folder by seeing if its path starts with the + * value stored here. + */ + const packages = new Map(); let usableByFileName: Path | undefined; const cache: ExportInfoMap = { isUsableByFile: importingFile => importingFile === usableByFileName, @@ -77,6 +89,29 @@ namespace ts { cache.clear(); usableByFileName = importingFile; } + + let packageName; + if (moduleFile) { + const nodeModulesPathParts = getNodeModulePathParts(moduleFile.fileName); + if (nodeModulesPathParts) { + const { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex } = nodeModulesPathParts; + packageName = unmangleScopedPackageName(getPackageNameFromTypesPackageName(moduleFile.fileName.substring(topLevelPackageNameIndex + 1, packageRootIndex))); + if (startsWith(importingFile, moduleFile.path.substring(0, topLevelNodeModulesIndex))) { + const prevDeepestNodeModulesPath = packages.get(packageName); + const nodeModulesPath = moduleFile.fileName.substring(0, topLevelPackageNameIndex); + if (prevDeepestNodeModulesPath) { + const prevDeepestNodeModulesIndex = prevDeepestNodeModulesPath.indexOf(nodeModulesPathPart); + if (topLevelNodeModulesIndex > prevDeepestNodeModulesIndex) { + packages.set(packageName, nodeModulesPath); + } + } + else { + packages.set(packageName, nodeModulesPath); + } + } + } + } + const isDefault = exportKind === ExportKind.Default; const namedSymbol = isDefault && getLocalSymbolForExportDefault(symbol) || symbol; // 1. A named export must be imported by its key in `moduleSymbol.exports` or `moduleSymbol.members`. @@ -104,6 +139,7 @@ namespace ts { moduleName, moduleFile, moduleFileName: moduleFile?.fileName, + packageName, exportKind, targetFlags: target.flags, isFromPackageJson, @@ -121,17 +157,20 @@ namespace ts { exportInfo.forEach((info, key) => { const { symbolName, ambientModuleName } = parseKey(key); const rehydrated = info.map(rehydrateCachedInfo); - action( - rehydrated, - preferCapitalized => { - const { symbol, exportKind } = rehydrated[0]; - const namedSymbol = exportKind === ExportKind.Default && getLocalSymbolForExportDefault(symbol) || symbol; - return preferCapitalized - ? getNameForExportedSymbol(namedSymbol, /*scriptTarget*/ undefined, /*preferCapitalized*/ true) - : symbolName; - }, - !!ambientModuleName, - key); + const filtered = rehydrated.filter((r, i) => isNotShadowedByDeeperNodeModulesPackage(r, info[i].packageName)); + if (filtered.length) { + action( + filtered, + preferCapitalized => { + const { symbol, exportKind } = rehydrated[0]; + const namedSymbol = exportKind === ExportKind.Default && getLocalSymbolForExportDefault(symbol) || symbol; + return preferCapitalized + ? getNameForExportedSymbol(namedSymbol, /*scriptTarget*/ undefined, /*preferCapitalized*/ true) + : symbolName; + }, + !!ambientModuleName, + key); + } }); }, releaseSymbols: () => { @@ -232,6 +271,12 @@ namespace ts { } return true; } + + function isNotShadowedByDeeperNodeModulesPackage(info: SymbolExportInfo, packageName: string | undefined) { + if (!packageName || !info.moduleFileName) return true; + const packageDeepestNodeModulesPath = packages.get(packageName); + return !packageDeepestNodeModulesPath || startsWith(info.moduleFileName, packageDeepestNodeModulesPath); + } } export function isImportableFile( diff --git a/tests/cases/fourslash/completionsImport_duplicatePackages_scoped.ts b/tests/cases/fourslash/completionsImport_duplicatePackages_scoped.ts new file mode 100644 index 0000000000000..567757ffac39f --- /dev/null +++ b/tests/cases/fourslash/completionsImport_duplicatePackages_scoped.ts @@ -0,0 +1,56 @@ +/// + +// @module: commonjs +// @esModuleInterop: true + +// @Filename: /node_modules/@scope/react-dom/package.json +//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@scope/react-dom/index.d.ts +//// import * as React from "react"; +//// export function render(): void; + +// @Filename: /node_modules/@scope/react/package.json +//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@scope/react/index.d.ts +//// import "./other"; +//// export declare function useState(): void; + +// @Filename: /node_modules/@scope/react/other.d.ts +//// export declare function useRef(): void; + +// @Filename: /packages/a/node_modules/@scope/react/package.json +//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" } + +// @Filename: /packages/a/node_modules/@scope/react/index.d.ts +//// export declare function useState(): void; + +// @Filename: /packages/a/index.ts +//// import "@scope/react-dom"; +//// import "@scope/react"; + +// @Filename: /packages/a/foo.ts +//// /**/ + +goTo.marker(""); +verify.completions({ + marker: "", + exact: completion.globalsPlus([{ + name: "render", + source: "@scope/react-dom", + sourceDisplay: "@scope/react-dom", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }, { + name: "useState", + source: "@scope/react", + sourceDisplay: "@scope/react", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }]), + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypes.ts b/tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypes.ts new file mode 100644 index 0000000000000..970e6cde0a3fc --- /dev/null +++ b/tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypes.ts @@ -0,0 +1,56 @@ +/// + +// @module: commonjs +// @esModuleInterop: true + +// @Filename: /node_modules/@types/scope__react-dom/package.json +//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/scope__react-dom/index.d.ts +//// import * as React from "react"; +//// export function render(): void; + +// @Filename: /node_modules/@types/scope__react/package.json +//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/scope__react/index.d.ts +//// import "./other"; +//// export declare function useState(): void; + +// @Filename: /node_modules/@types/scope__react/other.d.ts +//// export declare function useRef(): void; + +// @Filename: /packages/a/node_modules/@types/scope__react/package.json +//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" } + +// @Filename: /packages/a/node_modules/@types/scope__react/index.d.ts +//// export declare function useState(): void; + +// @Filename: /packages/a/index.ts +//// import "@scope/react-dom"; +//// import "@scope/react"; + +// @Filename: /packages/a/foo.ts +//// /**/ + +goTo.marker(""); +verify.completions({ + marker: "", + exact: completion.globalsPlus([{ + name: "render", + source: "@scope/react-dom", + sourceDisplay: "@scope/react-dom", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }, { + name: "useState", + source: "@scope/react", + sourceDisplay: "@scope/react", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }]), + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypesAndNotTypes.ts b/tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypesAndNotTypes.ts new file mode 100644 index 0000000000000..116d3145fc178 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypesAndNotTypes.ts @@ -0,0 +1,56 @@ +/// + +// @module: commonjs +// @esModuleInterop: true + +// @Filename: /node_modules/@types/scope__react-dom/package.json +//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/scope__react-dom/index.d.ts +//// import * as React from "react"; +//// export function render(): void; + +// @Filename: /node_modules/@types/scope__react/package.json +//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/scope__react/index.d.ts +//// import "./other"; +//// export declare function useState(): void; + +// @Filename: /node_modules/@types/scope__react/other.d.ts +//// export declare function useRef(): void; + +// @Filename: /packages/a/node_modules/@scope/react/package.json +//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" } + +// @Filename: /packages/a/node_modules/@scope/react/index.d.ts +//// export declare function useState(): void; + +// @Filename: /packages/a/index.ts +//// import "react-dom"; +//// import "react"; + +// @Filename: /packages/a/foo.ts +//// /**/ + +goTo.marker(""); +verify.completions({ + marker: "", + exact: completion.globalsPlus([{ + name: "render", + source: "@scope/react-dom", + sourceDisplay: "@scope/react-dom", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }, { + name: "useState", + source: "@scope/react", + sourceDisplay: "@scope/react", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }]), + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionsImport_duplicatePackages_types.ts b/tests/cases/fourslash/completionsImport_duplicatePackages_types.ts new file mode 100644 index 0000000000000..f29a52c018890 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_duplicatePackages_types.ts @@ -0,0 +1,57 @@ +/// + +// @module: commonjs +// @esModuleInterop: true + +// @Filename: /node_modules/@types/react-dom/package.json +//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/react-dom/index.d.ts +//// import * as React from "react"; +//// export function render(): void; + +// @Filename: /node_modules/@types/react/package.json +//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/react/index.d.ts +//// import "./other"; +//// export declare function useState(): void; + +// @Filename: /node_modules/@types/react/other.d.ts +//// export declare function useRef(): void; + +// @Filename: /packages/a/node_modules/@types/react/package.json +//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" } + +// @Filename: /packages/a/node_modules/@types/react/index.d.ts +//// export declare function useState(): void; + +// @Filename: /packages/a/index.ts +//// import "react-dom"; +//// import "react"; + +// @Filename: /packages/a/foo.ts +//// /**/ + +goTo.marker(""); +verify.completions({ + marker: "", + exact: completion.globalsPlus([{ + name: "render", + source: "react-dom", + sourceDisplay: "react-dom", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }, { + name: "useState", + source: "react", + sourceDisplay: "react", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }]), + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true, + }, +}); + diff --git a/tests/cases/fourslash/completionsImport_duplicatePackages_typesAndNotTypes.ts b/tests/cases/fourslash/completionsImport_duplicatePackages_typesAndNotTypes.ts new file mode 100644 index 0000000000000..bf9218cd82723 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_duplicatePackages_typesAndNotTypes.ts @@ -0,0 +1,50 @@ +/// + +// @module: commonjs +// @esModuleInterop: true + +// @Filename: /node_modules/@types/react-dom/package.json +//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/react-dom/index.d.ts +//// import * as React from "react"; +//// export function render(): void; + +// @Filename: /node_modules/@types/react/package.json +//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" } + +// @Filename: /node_modules/@types/react/index.d.ts +//// import "./other"; +//// export declare function useState(): void; + +// @Filename: /node_modules/@types/react/other.d.ts +//// export declare function useRef(): void; + +// @Filename: /packages/a/node_modules/react/package.json +//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" } + +// @Filename: /packages/a/node_modules/react/index.d.ts +//// export declare function useState(): void; + +// @Filename: /packages/a/index.ts +//// import "react-dom"; +//// import "react"; + +// @Filename: /packages/a/foo.ts +//// useState/**/ + +goTo.marker(""); +verify.completions({ + marker: "", + exact: completion.globalsPlus([{ + name: "useState", + source: "react", + sourceDisplay: "react", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions, + }]), + preferences: { + includeCompletionsForModuleExports: true, + allowIncompleteCompletions: true, + }, +});