From 3adcbd930bb7984754065a1508ecd9b2f741cd89 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 24 Jan 2022 15:59:50 -0800 Subject: [PATCH 1/4] Fix duplicate completions from two different copies of a node_modules package --- src/services/exportInfoMap.ts | 48 +++++++++++++++++- .../completionsImport_duplicatePackages.ts | 50 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionsImport_duplicatePackages.ts diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts index 2a43faf326965..c686d380f8083 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,30 @@ namespace ts { cache.clear(); usableByFileName = importingFile; } + + let packageName; + if (moduleFile) { + const nodeModulesIndex = moduleFile.fileName.indexOf(nodeModulesPathPart); + if (nodeModulesIndex !== -1) { + const isTypes = moduleFile.fileName.substring(nodeModulesIndex + nodeModulesPathPart.length).indexOf("@types/") === 0; + const nextSlash = moduleFile.fileName.indexOf(directorySeparator, nodeModulesIndex + nodeModulesPathPart.length + (isTypes ? "@types/".length : 0) + 1); + packageName = moduleFile.fileName.substring(nodeModulesIndex + nodeModulesPathPart.length + (isTypes ? "@types/".length : 0), nextSlash); + if (startsWith(importingFile, moduleFile.path.substring(0, nodeModulesIndex))) { + const prevDeepestNodeModulesPath = packages.get(packageName); + const nodeModulesPath = moduleFile.fileName.substring(0, nodeModulesIndex + nodeModulesPathPart.length); + if (prevDeepestNodeModulesPath) { + const prevDeepestNodeModulesIndex = prevDeepestNodeModulesPath.indexOf(nodeModulesPathPart); + if (nodeModulesIndex > 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`. @@ -103,6 +139,7 @@ namespace ts { moduleName, moduleFile, moduleFileName: moduleFile?.fileName, + packageName, exportKind, targetFlags: target.flags, isFromPackageJson, @@ -119,7 +156,10 @@ namespace ts { if (importingFile !== usableByFileName) return; exportInfo.forEach((info, key) => { const { symbolName, ambientModuleName } = parseKey(key); - action(info.map(rehydrateCachedInfo), symbolName, !!ambientModuleName, key); + const filtered = info.filter(isNotShadowedByDeeperNodeModulesPackage); + if (filtered.length) { + action(filtered.map(rehydrateCachedInfo), symbolName, !!ambientModuleName, key); + } }); }, releaseSymbols: () => { @@ -220,6 +260,12 @@ namespace ts { } return true; } + + function isNotShadowedByDeeperNodeModulesPackage(info: CachedSymbolExportInfo) { + if (!info.packageName || !info.moduleFileName) return true; + const packageDeepestNodeModulesPath = packages.get(info.packageName); + return !packageDeepestNodeModulesPath || startsWith(info.moduleFileName, packageDeepestNodeModulesPath); + } } export function isImportableFile( diff --git a/tests/cases/fourslash/completionsImport_duplicatePackages.ts b/tests/cases/fourslash/completionsImport_duplicatePackages.ts new file mode 100644 index 0000000000000..59545b56fd5ff --- /dev/null +++ b/tests/cases/fourslash/completionsImport_duplicatePackages.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/@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 +//// 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, + }, +}); From 91bdafb1248099d9e92d910e47e43e92930d6ef6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 27 Jan 2022 10:55:07 -0800 Subject: [PATCH 2/4] Fix logic for scoped packages --- src/compiler/moduleSpecifiers.ts | 64 ------------------- src/compiler/utilities.ts | 64 +++++++++++++++++++ src/services/exportInfoMap.ts | 15 ++--- ...pletionsImport_duplicatePackages_scoped.ts | 56 ++++++++++++++++ ...onsImport_duplicatePackages_scopedTypes.ts | 56 ++++++++++++++++ ...uplicatePackages_scopedTypesAndNotTypes.ts | 56 ++++++++++++++++ ...mpletionsImport_duplicatePackages_types.ts | 57 +++++++++++++++++ ...ort_duplicatePackages_typesAndNotTypes.ts} | 4 +- 8 files changed, 298 insertions(+), 74 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_duplicatePackages_scoped.ts create mode 100644 tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypes.ts create mode 100644 tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypesAndNotTypes.ts create mode 100644 tests/cases/fourslash/completionsImport_duplicatePackages_types.ts rename tests/cases/fourslash/{completionsImport_duplicatePackages.ts => completionsImport_duplicatePackages_typesAndNotTypes.ts} (91%) 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 691a965ee661d..ca4f6da7df350 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -7488,4 +7488,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 c686d380f8083..982b3fd2bf13c 100644 --- a/src/services/exportInfoMap.ts +++ b/src/services/exportInfoMap.ts @@ -92,17 +92,16 @@ namespace ts { let packageName; if (moduleFile) { - const nodeModulesIndex = moduleFile.fileName.indexOf(nodeModulesPathPart); - if (nodeModulesIndex !== -1) { - const isTypes = moduleFile.fileName.substring(nodeModulesIndex + nodeModulesPathPart.length).indexOf("@types/") === 0; - const nextSlash = moduleFile.fileName.indexOf(directorySeparator, nodeModulesIndex + nodeModulesPathPart.length + (isTypes ? "@types/".length : 0) + 1); - packageName = moduleFile.fileName.substring(nodeModulesIndex + nodeModulesPathPart.length + (isTypes ? "@types/".length : 0), nextSlash); - if (startsWith(importingFile, moduleFile.path.substring(0, nodeModulesIndex))) { + 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, nodeModulesIndex + nodeModulesPathPart.length); + const nodeModulesPath = moduleFile.fileName.substring(0, topLevelPackageNameIndex); if (prevDeepestNodeModulesPath) { const prevDeepestNodeModulesIndex = prevDeepestNodeModulesPath.indexOf(nodeModulesPathPart); - if (nodeModulesIndex > prevDeepestNodeModulesIndex) { + if (topLevelNodeModulesIndex > prevDeepestNodeModulesIndex) { packages.set(packageName, nodeModulesPath); } } 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.ts b/tests/cases/fourslash/completionsImport_duplicatePackages_typesAndNotTypes.ts similarity index 91% rename from tests/cases/fourslash/completionsImport_duplicatePackages.ts rename to tests/cases/fourslash/completionsImport_duplicatePackages_typesAndNotTypes.ts index 59545b56fd5ff..bf9218cd82723 100644 --- a/tests/cases/fourslash/completionsImport_duplicatePackages.ts +++ b/tests/cases/fourslash/completionsImport_duplicatePackages_typesAndNotTypes.ts @@ -20,10 +20,10 @@ // @Filename: /node_modules/@types/react/other.d.ts //// export declare function useRef(): void; -// @Filename: /packages/a/node_modules/@types/react/package.json +// @Filename: /packages/a/node_modules/react/package.json //// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" } -// @Filename: /packages/a/node_modules/@types/react/index.d.ts +// @Filename: /packages/a/node_modules/react/index.d.ts //// export declare function useState(): void; // @Filename: /packages/a/index.ts From 0c5a3363949a5a571a21bfa096279a2eb80a4b14 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 27 Jan 2022 11:33:17 -0800 Subject: [PATCH 3/4] Fix errors from merge --- src/services/codefixes/importFixes.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 3c1dc5026ac86..1074b4fa0a6ba 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -350,11 +350,11 @@ namespace ts.codefix { function getInfoWithChecker(checker: TypeChecker, isFromPackageJson: boolean): SymbolExportInfo | undefined { const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); if (defaultInfo && skipAlias(defaultInfo.symbol, checker) === symbol) { - return { symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: undefined, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson }; + return { symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: undefined, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson, packageName: undefined }; } const named = checker.tryGetMemberInModuleExportsAndProperties(symbol.name, moduleSymbol); if (named && skipAlias(named, checker) === symbol) { - return { symbol: named, moduleSymbol, moduleFileName: undefined, exportKind: ExportKind.Named, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson }; + return { symbol: named, moduleSymbol, moduleFileName: undefined, exportKind: ExportKind.Named, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson, packageName: undefined }; } } } @@ -375,12 +375,12 @@ namespace ts.codefix { const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions), isJsxTagName) === symbolName) && skipAlias(defaultInfo.symbol, checker) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) { - result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson }); + result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson, packageName: undefined }); } for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) { if (exported.name === symbolName && checker.getMergedSymbol(skipAlias(exported, checker)) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) { - result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson }); + result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson, packageName: undefined }); } } }); @@ -742,7 +742,7 @@ namespace ts.codefix { if (!umdSymbol) return undefined; const symbol = checker.getAliasedSymbol(umdSymbol); const symbolName = umdSymbol.name; - const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, targetFlags: symbol.flags, isFromPackageJson: false }]; + const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, targetFlags: symbol.flags, isFromPackageJson: false, packageName: undefined }]; const useRequire = shouldUseRequire(sourceFile, program); const fixes = getImportFixes(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, /*isValidTypeOnlyUseSite*/ false, useRequire, program, sourceFile, host, preferences); return { fixes, symbolName, errorIdentifierText: tryCast(token, isIdentifier)?.text }; @@ -878,7 +878,7 @@ namespace ts.codefix { !toFile && packageJsonFilter.allowsImportingAmbientModule(moduleSymbol, moduleSpecifierResolutionHost) ) { const checker = program.getTypeChecker(); - originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, targetFlags: skipAlias(exportedSymbol, checker).flags, isFromPackageJson }); + originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, targetFlags: skipAlias(exportedSymbol, checker).flags, isFromPackageJson, packageName: undefined }); } } forEachExternalModuleToImportFrom(program, host, useAutoImportProvider, (moduleSymbol, sourceFile, program, isFromPackageJson) => { From 7b9c222f1c658ce6342fb37c46545333fa2f76f2 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 27 Jan 2022 14:05:27 -0800 Subject: [PATCH 4/4] Less gross way to reconcile these two conflicting PRs --- src/services/codefixes/importFixes.ts | 12 ++++++------ src/services/exportInfoMap.ts | 13 +++++-------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 1074b4fa0a6ba..3c1dc5026ac86 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -350,11 +350,11 @@ namespace ts.codefix { function getInfoWithChecker(checker: TypeChecker, isFromPackageJson: boolean): SymbolExportInfo | undefined { const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); if (defaultInfo && skipAlias(defaultInfo.symbol, checker) === symbol) { - return { symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: undefined, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson, packageName: undefined }; + return { symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: undefined, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson }; } const named = checker.tryGetMemberInModuleExportsAndProperties(symbol.name, moduleSymbol); if (named && skipAlias(named, checker) === symbol) { - return { symbol: named, moduleSymbol, moduleFileName: undefined, exportKind: ExportKind.Named, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson, packageName: undefined }; + return { symbol: named, moduleSymbol, moduleFileName: undefined, exportKind: ExportKind.Named, targetFlags: skipAlias(symbol, checker).flags, isFromPackageJson }; } } } @@ -375,12 +375,12 @@ namespace ts.codefix { const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions), isJsxTagName) === symbolName) && skipAlias(defaultInfo.symbol, checker) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) { - result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson, packageName: undefined }); + result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson }); } for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) { if (exported.name === symbolName && checker.getMergedSymbol(skipAlias(exported, checker)) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) { - result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson, packageName: undefined }); + result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson }); } } }); @@ -742,7 +742,7 @@ namespace ts.codefix { if (!umdSymbol) return undefined; const symbol = checker.getAliasedSymbol(umdSymbol); const symbolName = umdSymbol.name; - const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, targetFlags: symbol.flags, isFromPackageJson: false, packageName: undefined }]; + const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, targetFlags: symbol.flags, isFromPackageJson: false }]; const useRequire = shouldUseRequire(sourceFile, program); const fixes = getImportFixes(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, /*isValidTypeOnlyUseSite*/ false, useRequire, program, sourceFile, host, preferences); return { fixes, symbolName, errorIdentifierText: tryCast(token, isIdentifier)?.text }; @@ -878,7 +878,7 @@ namespace ts.codefix { !toFile && packageJsonFilter.allowsImportingAmbientModule(moduleSymbol, moduleSpecifierResolutionHost) ) { const checker = program.getTypeChecker(); - originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, targetFlags: skipAlias(exportedSymbol, checker).flags, isFromPackageJson, packageName: undefined }); + originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { symbol: exportedSymbol, moduleSymbol, moduleFileName: toFile?.fileName, exportKind, targetFlags: skipAlias(exportedSymbol, checker).flags, isFromPackageJson }); } } forEachExternalModuleToImportFrom(program, host, useAutoImportProvider, (moduleSymbol, sourceFile, program, isFromPackageJson) => { diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts index 95e96ce3bbcb8..a234e7376fa7d 100644 --- a/src/services/exportInfoMap.ts +++ b/src/services/exportInfoMap.ts @@ -23,7 +23,6 @@ namespace ts { targetFlags: SymbolFlags; /** True if export was only found via the package.json AutoImportProvider (for telemetry). */ isFromPackageJson: boolean; - packageName: string | undefined; } interface CachedSymbolExportInfo { @@ -158,7 +157,7 @@ namespace ts { exportInfo.forEach((info, key) => { const { symbolName, ambientModuleName } = parseKey(key); const rehydrated = info.map(rehydrateCachedInfo); - const filtered = rehydrated.filter(isNotShadowedByDeeperNodeModulesPackage); + const filtered = rehydrated.filter((r, i) => isNotShadowedByDeeperNodeModulesPackage(r, info[i].packageName)); if (filtered.length) { action( filtered, @@ -207,7 +206,7 @@ namespace ts { function rehydrateCachedInfo(info: CachedSymbolExportInfo): SymbolExportInfo { if (info.symbol && info.moduleSymbol) return info as SymbolExportInfo; - const { id, exportKind, targetFlags, isFromPackageJson, moduleFileName, packageName } = info; + const { id, exportKind, targetFlags, isFromPackageJson, moduleFileName } = info; const [cachedSymbol, cachedModuleSymbol] = symbols.get(id) || emptyArray; if (cachedSymbol && cachedModuleSymbol) { return { @@ -217,7 +216,6 @@ namespace ts { exportKind, targetFlags, isFromPackageJson, - packageName, }; } const checker = (isFromPackageJson @@ -238,7 +236,6 @@ namespace ts { exportKind, targetFlags, isFromPackageJson, - packageName, }; } @@ -275,9 +272,9 @@ namespace ts { return true; } - function isNotShadowedByDeeperNodeModulesPackage(info: SymbolExportInfo) { - if (!info.packageName || !info.moduleFileName) return true; - const packageDeepestNodeModulesPath = packages.get(info.packageName); + function isNotShadowedByDeeperNodeModulesPackage(info: SymbolExportInfo, packageName: string | undefined) { + if (!packageName || !info.moduleFileName) return true; + const packageDeepestNodeModulesPath = packages.get(packageName); return !packageDeepestNodeModulesPath || startsWith(info.moduleFileName, packageDeepestNodeModulesPath); } }