From ec34d03b30451b27b16aba9f8eafebc8b65c1e54 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 20 Jul 2018 11:44:26 -0700 Subject: [PATCH 1/4] importFixes: When one file redirects to another, consider both for global import specifiers --- src/compiler/checker.ts | 3 +- src/compiler/moduleSpecifiers.ts | 17 ++++-- src/compiler/program.ts | 11 ++-- src/compiler/types.ts | 9 +++- src/services/codefixes/importFixes.ts | 2 +- src/services/documentHighlights.ts | 2 +- src/services/getEditsForFileRename.ts | 2 +- .../unittests/tsserverProjectSystem.ts | 53 +++++++++++++++++++ 8 files changed, 83 insertions(+), 16 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e01c97af49222..8e4632cc8a76c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3890,7 +3890,8 @@ namespace ts { contextFile, context.tracker.moduleResolverHost, context.tracker.moduleResolverHost.getSourceFiles!(), // TODO: GH#18217 - { importModuleSpecifierPreference: "non-relative" } + { importModuleSpecifierPreference: "non-relative" }, + host.redirectTargetsMap, ))[0]; links.specifierCache = links.specifierCache || createMap(); links.specifierCache.set(contextFile.path, specifier); diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 11559815412e7..aeb2423e8f15d 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -14,9 +14,10 @@ namespace ts.moduleSpecifiers { host: ModuleSpecifierResolutionHost, files: ReadonlyArray, preferences: ModuleSpecifierPreferences = {}, + redirectTargetsMap: RedirectTargetsMap, ): string { const info = getInfo(compilerOptions, importingSourceFile, importingSourceFileName, host); - const modulePaths = getAllModulePaths(files, toFileName, info.getCanonicalFileName, host); + const modulePaths = getAllModulePaths(files, toFileName, info.getCanonicalFileName, host, redirectTargetsMap); return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)) || first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences)); } @@ -29,6 +30,7 @@ namespace ts.moduleSpecifiers { host: ModuleSpecifierResolutionHost, files: ReadonlyArray, preferences: ModuleSpecifierPreferences, + redirectTargetsMap: RedirectTargetsMap, ): ReadonlyArray> { const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol); if (ambient) return [[ambient]]; @@ -37,7 +39,8 @@ namespace ts.moduleSpecifiers { if (!files) { return Debug.fail("Files list must be present to resolve symlinks in specifier resolution"); } - const modulePaths = getAllModulePaths(files, getSourceFileOfNode(moduleSymbol.valueDeclaration).fileName, info.getCanonicalFileName, host); + const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration); + const modulePaths = getAllModulePaths(files, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap); const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)); return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName => @@ -190,11 +193,15 @@ namespace ts.moduleSpecifiers { * Looks for existing imports that use symlinks to this module. * Only if no symlink is available, the real path will be used. */ - function getAllModulePaths(files: ReadonlyArray, importedFileName: string, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost): ReadonlyArray { + function getAllModulePaths(files: ReadonlyArray, importedFileName: string, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): ReadonlyArray { + const redirects = redirectTargetsMap.get(importedFileName); + const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName]; const symlinks = mapDefined(files, sf => sf.resolvedModules && firstDefinedIterator(sf.resolvedModules.values(), res => - res && res.resolvedFileName === importedFileName ? res.originalPath : undefined)); - return symlinks.length === 0 ? getAllModulePathsUsingIndirectSymlinks(files, getNormalizedAbsolutePath(importedFileName, host.getCurrentDirectory ? host.getCurrentDirectory() : ""), getCanonicalFileName, host) : symlinks; + res && contains(importedFileNames, res.resolvedFileName) ? res.originalPath : undefined)); + return symlinks.length === 0 + ? flatMap(importedFileNames, importedFileName => getAllModulePathsUsingIndirectSymlinks(files, getNormalizedAbsolutePath(importedFileName, host.getCurrentDirectory ? host.getCurrentDirectory() : ""), getCanonicalFileName, host)) + : symlinks; } function getRelativePathNParents(relativePath: string): number { diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 334c054a8fb0e..ac9552d6db7a2 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -634,7 +634,8 @@ namespace ts { const packageIdToSourceFile = createMap(); // Maps from a SourceFile's `.path` to the name of the package it was imported with. let sourceFileToPackageName = createMap(); - let redirectTargetsSet = createMap(); + // Key is a file name. Value is the (non-empty, or undefined) list of files that redirect to it. + let redirectTargetsMap = createMultiMap(); const filesByName = createMap(); let missingFilePaths: ReadonlyArray | undefined; @@ -752,7 +753,7 @@ namespace ts { getSourceFileFromReference, getLibFileFromReference, sourceFileToPackageName, - redirectTargetsSet, + redirectTargetsMap, isEmittedFile, getConfigFileParsingDiagnostics, getResolvedModuleWithFailedLookupLocationsFromCache, @@ -1073,7 +1074,7 @@ namespace ts { fileChanged = false; newSourceFile = oldSourceFile; // Use the redirect. } - else if (oldProgram.redirectTargetsSet.has(oldSourceFile.path)) { + else if (oldProgram.redirectTargetsMap.has(oldSourceFile.path)) { // If a redirected-to source file changes, the redirect may be broken. if (newSourceFile !== oldSourceFile) { return oldProgram.structureIsReused = StructureIsReused.Not; @@ -1220,7 +1221,7 @@ namespace ts { resolvedProjectReferences = oldProgram.getProjectReferences(); sourceFileToPackageName = oldProgram.sourceFileToPackageName; - redirectTargetsSet = oldProgram.redirectTargetsSet; + redirectTargetsMap = oldProgram.redirectTargetsMap; return oldProgram.structureIsReused = StructureIsReused.Completely; } @@ -2071,7 +2072,7 @@ namespace ts { // Some other SourceFile already exists with this package name and version. // Instead of creating a duplicate, just redirect to the existing one. const dupFile = createRedirectSourceFile(fileFromPackageId, file!, fileName, path); // TODO: GH#18217 - redirectTargetsSet.set(fileFromPackageId.path, true); + redirectTargetsMap.add(fileFromPackageId.path, fileName); filesByName.set(path, dupFile); sourceFileToPackageName.set(path, packageId.name); processingOtherFiles!.push(dupFile); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 9107233533009..d301f6dae9a5e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2543,7 +2543,7 @@ namespace ts { /** * If two source files are for the same version of the same package, one will redirect to the other. * (See `createRedirectSourceFile` in program.ts.) - * The redirect will have this set. The redirected-to source file will be in `redirectTargetsSet`. + * The redirect will have this set. The redirected-to source file will be in `redirectTargetsMap`. */ /* @internal */ redirectInfo?: RedirectInfo; @@ -2790,7 +2790,7 @@ namespace ts { /** Given a source file, get the name of the package it was imported from. */ /* @internal */ sourceFileToPackageName: Map; /** Set of all source files that some other source file redirects to. */ - /* @internal */ redirectTargetsSet: Map; + /* @internal */ redirectTargetsMap: MultiMap; /** Is the file emitted file */ /* @internal */ isEmittedFile(file: string): boolean; @@ -2799,6 +2799,9 @@ namespace ts { getProjectReferences(): (ResolvedProjectReference | undefined)[] | undefined; } + /* @internal */ + export type RedirectTargetsMap = ReadonlyMap>; + export interface ResolvedProjectReference { commandLine: ParsedCommandLine; sourceFile: SourceFile; @@ -2876,6 +2879,8 @@ namespace ts { getSourceFiles(): ReadonlyArray; getSourceFile(fileName: string): SourceFile | undefined; getResolvedTypeReferenceDirectives(): ReadonlyMap; + + readonly redirectTargetsMap: RedirectTargetsMap; } export interface TypeChecker { diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 38a8ab78418cb..2d034c4ca958b 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -254,7 +254,7 @@ namespace ts.codefix { preferences: UserPreferences, ): ReadonlyArray { const choicesForEachExportingModule = flatMap>(moduleSymbols, ({ moduleSymbol, importKind }) => { - const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences); + const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences, program.redirectTargetsMap); return modulePathsGroups.map(group => group.map((moduleSpecifier): FixAddNewImport => ({ kind: ImportFixKind.AddNew, moduleSpecifier, importKind }))); }); // Sort to keep the shortest paths first, but keep [relativePath, importRelativeToBaseUrl] groups together diff --git a/src/services/documentHighlights.ts b/src/services/documentHighlights.ts index 6976cd2ea2001..f0f3c165d7eab 100644 --- a/src/services/documentHighlights.ts +++ b/src/services/documentHighlights.ts @@ -28,7 +28,7 @@ namespace ts.DocumentHighlights { const map = arrayToMultiMap(referenceEntries.map(FindAllReferences.toHighlightSpan), e => e.fileName, e => e.span); return arrayFrom(map.entries(), ([fileName, highlightSpans]) => { if (!sourceFilesSet.has(fileName)) { - Debug.assert(program.redirectTargetsSet.has(fileName)); + Debug.assert(program.redirectTargetsMap.has(fileName)); const redirectTarget = program.getSourceFile(fileName); const redirect = find(sourceFilesToSearch, f => !!f.redirectInfo && f.redirectInfo.redirectTarget === redirectTarget)!; fileName = redirect.fileName; diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts index 75dd608e6c475..bd00ce5dd9e23 100644 --- a/src/services/getEditsForFileRename.ts +++ b/src/services/getEditsForFileRename.ts @@ -156,7 +156,7 @@ namespace ts { // Need an update if the imported file moved, or the importing file moved and was using a relative path. return toImport !== undefined && (toImport.updated || (importingSourceFileMoved && pathIsRelative(importLiteral.text))) - ? moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport.newFileName, host, allFiles, preferences) + ? moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport.newFileName, host, allFiles, preferences, program.redirectTargetsMap) : undefined; }); } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index ff5ee24bf5d27..7ecab3123119b 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -9260,6 +9260,59 @@ export function Test2() { }); }); + describe("duplicate packages", () => { + // Tests that 'moduleSpecifiers.ts' will import from the redirecting file, and not from the file it redirects to, if that can provide a global module specifier. + it("works with import fixes", () => { + const packageContent = "export const foo: number;"; + const packageJsonContent = JSON.stringify({ name: "foo", version: "1.2.3" }); + const aFooIndex: File = { path: "/a/node_modules/foo/index.d.ts", content: packageContent }; + const aFooPackage: File = { path: "/a/node_modules/foo/package.json", content: packageJsonContent }; + const bFooIndex: File = { path: "/b/node_modules/foo/index.d.ts", content: packageContent }; + const bFooPackage: File = { path: "/b/node_modules/foo/package.json", content: packageJsonContent }; + + const userContent = 'import("foo");\nfoo'; + const aUser: File = { path: "/a/user.ts", content: userContent }; + const bUser: File = { path: "/b/user.ts", content: userContent }; + const tsconfig: File = { + path: "/tsconfig.json", + content: "{}", + }; + + const host = createServerHost([aFooIndex, aFooPackage, bFooIndex, bFooPackage, aUser, bUser, tsconfig]); + const session = createSession(host); + + openFilesForSession([aUser, bUser], session); + + for (const user of [aUser, bUser]) { + const response = executeSessionRequest(session, protocol.CommandTypes.GetCodeFixes, { + file: user.path, + startLine: 2, + startOffset: 1, + endLine: 2, + endOffset: 4, + errorCodes: [Diagnostics.Cannot_find_name_0.code], + }); + assert.deepEqual | undefined>(response, [ + { + description: `Import 'foo' from module "foo"`, + fixName: "import", + fixId: "fixMissingImport", + fixAllDescription: "Add all missing imports", + changes: [{ + fileName: user.path, + textChanges: [{ + start: { line: 1, offset: 1 }, + end: { line: 1, offset: 1 }, + newText: 'import { foo } from "foo";\n\n', + }], + }], + commands: undefined, + }, + ]); + } + }); + }); + describe("Untitled files", () => { it("Can convert positions to locations", () => { const aTs: File = { path: "/proj/a.ts", content: "" }; From d9b285cadcc3b72bdf39d624d1d86308cc9be67d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 30 Jul 2018 11:09:46 -0700 Subject: [PATCH 2/4] Add test for #26044 --- .../importNameCodeFix_symlink_own_package.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/cases/fourslash/importNameCodeFix_symlink_own_package.ts diff --git a/tests/cases/fourslash/importNameCodeFix_symlink_own_package.ts b/tests/cases/fourslash/importNameCodeFix_symlink_own_package.ts new file mode 100644 index 0000000000000..67dfd71fff1b6 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_symlink_own_package.ts @@ -0,0 +1,21 @@ +/// + +// @Filename: /packages/b/b0.ts +// @Symlink: /node_modules/b/b0.ts +////x; + +// @Filename: /packages/b/b1.ts +// @Symlink: /node_modules/b/b1.ts +////import { a } from "a"; +////export const x = 0; + +// @Filename: /packages/a/index.d.ts +// @Symlink: /node_modules/a/index.d.ts +////export const a: number; + +goTo.file("/packages/b/b0.ts"); +verify.importFixAtPosition([ +`import { x } from "./b1"; + +x;`, +]); From 15b8af7d885357d81ab96766031faeb9bcf3dba6 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 30 Jul 2018 11:41:58 -0700 Subject: [PATCH 3/4] Avoid a symlinked package globally importing itself (fixes another case of #26044) --- src/compiler/checker.ts | 4 +-- src/compiler/moduleSpecifiers.ts | 33 +++++++++++++++---- src/compiler/utilities.ts | 6 ++++ ...importNameCodeFix_symlink_own_package_2.ts | 17 ++++++++++ 4 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFix_symlink_own_package_2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6c99ab7c9664a..52729c1e8fb5a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3883,7 +3883,7 @@ namespace ts { const links = getSymbolLinks(symbol); let specifier = links.specifierCache && links.specifierCache.get(contextFile.path); if (!specifier) { - specifier = flatten(moduleSpecifiers.getModuleSpecifiers( + specifier = moduleSpecifiers.getModuleSpecifierForDeclarationFile( symbol, compilerOptions, contextFile, @@ -3891,7 +3891,7 @@ namespace ts { context.tracker.moduleResolverHost.getSourceFiles!(), // TODO: GH#18217 { importModuleSpecifierPreference: "non-relative" }, host.redirectTargetsMap, - ))[0]; + ); links.specifierCache = links.specifierCache || createMap(); links.specifierCache.set(contextFile.path, specifier); } diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 0237a1e1fd830..e292603faf833 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -1,4 +1,4 @@ -// Used by importFixes to synthesize import module specifiers. +// Used by importFixes, getEditsForFileRename, and declaration emit to synthesize import module specifiers. /* @internal */ namespace ts.moduleSpecifiers { export interface ModuleSpecifierPreferences { @@ -17,10 +17,22 @@ namespace ts.moduleSpecifiers { redirectTargetsMap: RedirectTargetsMap, ): string { const info = getInfo(compilerOptions, importingSourceFile, importingSourceFileName, host); - const modulePaths = getAllModulePaths(files, toFileName, info.getCanonicalFileName, host, redirectTargetsMap); + const modulePaths = getAllModulePaths(files, importingSourceFileName, toFileName, info.getCanonicalFileName, host, redirectTargetsMap); return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)) || first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences)); } + + export function getModuleSpecifierForDeclarationFile( + moduleSymbol: Symbol, + compilerOptions: CompilerOptions, + importingSourceFile: SourceFile, + host: ModuleSpecifierResolutionHost, + files: ReadonlyArray, + preferences: ModuleSpecifierPreferences, + redirectTargetsMap: RedirectTargetsMap, + ): string { + return first(first(getModuleSpecifiers(moduleSymbol, compilerOptions, importingSourceFile, host, files, preferences, redirectTargetsMap))); + } // For each symlink/original for a module, returns a list of ways to import that file. export function getModuleSpecifiers( @@ -40,7 +52,7 @@ namespace ts.moduleSpecifiers { return Debug.fail("Files list must be present to resolve symlinks in specifier resolution"); } const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration); - const modulePaths = getAllModulePaths(files, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap); + const modulePaths = getAllModulePaths(files, importingSourceFile.path, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap); const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)); return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName => @@ -149,11 +161,16 @@ namespace ts.moduleSpecifiers { return getCanonicalFileName(a) === getCanonicalFileName(b); } + // KLUDGE: Don't assume one 'node_modules' links to another. More likely a single directory inside the node_modules is the symlink. + // ALso, don't assume that an `@foo` directory is linked. More likely the contents of that are linked. + function isNodeModulesOrScopedPackageDirectory(s: string): boolean { + return s === "node_modules" || startsWith(s, "@"); + } + function guessDirectorySymlink(a: string, b: string, cwd: string, getCanonicalFileName: GetCanonicalFileName): [string, string] { const aParts = getPathComponents(toPath(a, cwd, getCanonicalFileName)); const bParts = getPathComponents(toPath(b, cwd, getCanonicalFileName)); - // KLUDGE: Don't assume one 'node_modules' links to another. More likely a single directory inside the node_modules is the symlink. - while ((aParts[aParts.length - 2]) !== "node_modules" && bParts[bParts.length - 2] !== "node_modules" && stringsEqual(aParts[aParts.length - 1], bParts[bParts.length - 1], getCanonicalFileName)) { + while (!isNodeModulesOrScopedPackageDirectory(aParts[aParts.length - 2]) && !isNodeModulesOrScopedPackageDirectory(bParts[bParts.length - 2]) && stringsEqual(aParts[aParts.length - 1], bParts[bParts.length - 1], getCanonicalFileName)) { aParts.pop(); bParts.pop(); } @@ -176,7 +193,7 @@ namespace ts.moduleSpecifiers { * Looks for existing imports that use symlinks to this module. * Symlinks will be returned first so they are preferred over the real path. */ - function getAllModulePaths(files: ReadonlyArray, importedFileName: string, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): ReadonlyArray { + function getAllModulePaths(files: ReadonlyArray, importingFileName: string, importedFileName: string, getCanonicalFileName: GetCanonicalFileName, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): ReadonlyArray { const redirects = redirectTargetsMap.get(importedFileName); const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName]; const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : ""; @@ -186,6 +203,10 @@ namespace ts.moduleSpecifiers { const result: string[] = []; const compareStrings = (!host.useCaseSensitiveFileNames || host.useCaseSensitiveFileNames()) ? compareStringsCaseSensitive : compareStringsCaseInsensitive; links.forEach((resolved, path) => { + if (startsWithDirectory(importingFileName, resolved, getCanonicalFileName)) { + return; // Don't want to a package to globally import from itself + } + const target = targets.find(t => compareStrings(t.slice(0, resolved.length + 1), resolved + "/") === Comparison.EqualTo); if (target === undefined) return; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 9053d0cb83e6f..c835df2fb47a8 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -7157,6 +7157,12 @@ namespace ts { return path.slice(0, Math.max(rootLength, path.lastIndexOf(directorySeparator))); } + export function startsWithDirectory(fileName: string, directoryName: string, getCanonicalFileName: GetCanonicalFileName): boolean { + const canonicalFileName = getCanonicalFileName(fileName); + const canonicalDirectoryName = getCanonicalFileName(directoryName); + return startsWith(canonicalFileName, canonicalDirectoryName + "/") || startsWith(canonicalFileName, canonicalDirectoryName + "\\"); + } + export function isUrl(path: string) { return getEncodedRootLength(path) < 0; } diff --git a/tests/cases/fourslash/importNameCodeFix_symlink_own_package_2.ts b/tests/cases/fourslash/importNameCodeFix_symlink_own_package_2.ts new file mode 100644 index 0000000000000..f1dec143d34e2 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_symlink_own_package_2.ts @@ -0,0 +1,17 @@ +/// + +// @Filename: /packages/a/test.ts +// @Symlink: /node_modules/a/test.ts +////x; + +// @Filename: /packages/a/utils.ts +// @Symlink: /node_modules/a/utils.ts +////import {} from "a/utils"; +////export const x = 0; + +goTo.file("/packages/a/test.ts"); +verify.importFixAtPosition([ +`import { x } from "./utils"; + +x;`, +]); From f9ae9735ee738b4eb5fd1eff9d79555e80d72ee3 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 31 Jul 2018 16:46:27 -0700 Subject: [PATCH 4/4] Compare to node_modules with getCanonicalFileName --- src/compiler/moduleSpecifiers.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index b68a48e5f16e3..a82f5b854e12a 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -163,14 +163,16 @@ namespace ts.moduleSpecifiers { // KLUDGE: Don't assume one 'node_modules' links to another. More likely a single directory inside the node_modules is the symlink. // ALso, don't assume that an `@foo` directory is linked. More likely the contents of that are linked. - function isNodeModulesOrScopedPackageDirectory(s: string): boolean { - return s === "node_modules" || startsWith(s, "@"); + function isNodeModulesOrScopedPackageDirectory(s: string, getCanonicalFileName: GetCanonicalFileName): boolean { + return getCanonicalFileName(s) === "node_modules" || startsWith(s, "@"); } function guessDirectorySymlink(a: string, b: string, cwd: string, getCanonicalFileName: GetCanonicalFileName): [string, string] { const aParts = getPathComponents(toPath(a, cwd, getCanonicalFileName)); const bParts = getPathComponents(toPath(b, cwd, getCanonicalFileName)); - while (!isNodeModulesOrScopedPackageDirectory(aParts[aParts.length - 2]) && !isNodeModulesOrScopedPackageDirectory(bParts[bParts.length - 2]) && stringsEqual(aParts[aParts.length - 1], bParts[bParts.length - 1], getCanonicalFileName)) { + while (!isNodeModulesOrScopedPackageDirectory(aParts[aParts.length - 2], getCanonicalFileName) && + !isNodeModulesOrScopedPackageDirectory(bParts[bParts.length - 2], getCanonicalFileName) && + stringsEqual(aParts[aParts.length - 1], bParts[bParts.length - 1], getCanonicalFileName)) { aParts.pop(); bParts.pop(); }