From 2bb56dafe74365af25af2e9b4e2c050c903b0f98 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 9 Dec 2022 16:41:15 -0800 Subject: [PATCH] Always respect preserveSymlinks --- src/compiler/moduleNameResolver.ts | 74 +++++++++++++------ ...olutionWithExtensions_withPaths.trace.json | 2 + ...es_one_externalModule_withPaths.trace.json | 3 + ...ithExtension_MapedToNodeModules.trace.json | 1 + .../requireOfJsonFile_PathMapping.trace.json | 1 + .../scopedPackagesClassic.trace.json | 1 + .../reference/typingsLookupAmd.trace.json | 2 + 7 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 215a08000ea1a..72f1b7bfb8b99 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -201,6 +201,37 @@ function resolvedTypeScriptOnly(resolved: Resolved | undefined): PathAndPackageI return { fileName: resolved.path, packageId: resolved.packageId }; } +function createResolvedModuleWithFailedLookupLocationsHandlingSymlink( + moduleName: string, + resolved: Resolved | undefined, + isExternalLibraryImport: boolean | undefined, + failedLookupLocations: string[], + affectingLocations: string[], + diagnostics: Diagnostic[], + state: ModuleResolutionState, +): ResolvedModuleWithFailedLookupLocations { + // If this is from node_modules for non relative name, always respect preserveSymlinks + if (!state.resultFromCache && + !state.compilerOptions.preserveSymlinks && + resolved && + isExternalLibraryImport && + !resolved.originalPath && + !isExternalModuleNameRelative(moduleName) + ) { + const { resolvedFileName, originalPath } = getOriginalAndResolvedFileName(resolved.path, state.host, state.traceEnabled); + if (originalPath) resolved = { ...resolved, path: resolvedFileName, originalPath }; + } + return createResolvedModuleWithFailedLookupLocations( + state.compilerOptions, + resolved, + isExternalLibraryImport, + failedLookupLocations, + affectingLocations, + diagnostics, + state.resultFromCache, + ); +} + function createResolvedModuleWithFailedLookupLocations( compilerOptions: CompilerOptions, resolved: Resolved | undefined, @@ -436,6 +467,16 @@ function arePathsEqual(path1: string, path2: string, host: ModuleResolutionHost) return comparePaths(path1, path2, !useCaseSensitiveFileNames) === Comparison.EqualTo; } +function getOriginalAndResolvedFileName(fileName: string, host: ModuleResolutionHost, traceEnabled: boolean) { + const resolvedFileName = realPath(fileName, host, traceEnabled); + const pathsAreEqual = arePathsEqual(fileName, resolvedFileName, host); + return { + // If the fileName and realpath are differing only in casing prefer fileName so that we can issue correct errors for casing under forceConsistentCasingInFileNames + resolvedFileName: pathsAreEqual ? fileName : resolvedFileName, + originalPath: pathsAreEqual ? undefined : fileName, + }; +} + /** * @param {string | undefined} containingFile - file that contains type reference directive, can be undefined if containing file is unknown. * This is possible in case if resolution is performed for directives specified via 'types' parameter. In this case initial path for secondary lookups @@ -526,13 +567,12 @@ export function resolveTypeReferenceDirective(typeReferenceDirectiveName: string let resolvedTypeReferenceDirective: ResolvedTypeReferenceDirective | undefined; if (resolved) { const { fileName, packageId } = resolved; - const resolvedFileName = options.preserveSymlinks ? fileName : realPath(fileName, host, traceEnabled); - const pathsAreEqual = arePathsEqual(fileName, resolvedFileName, host); + let resolvedFileName = fileName, originalPath: string | undefined; + if (!options.preserveSymlinks) ({ resolvedFileName, originalPath } = getOriginalAndResolvedFileName(fileName, host, traceEnabled)); resolvedTypeReferenceDirective = { primary, - // If the fileName and realpath are differing only in casing prefer fileName so that we can issue correct errors for casing under forceConsistentCasingInFileNames - resolvedFileName: pathsAreEqual ? fileName : resolvedFileName, - originalPath: pathsAreEqual ? undefined : fileName, + resolvedFileName, + originalPath, packageId, isExternalLibraryImport: pathContainsNodeModules(fileName), }; @@ -1792,14 +1832,14 @@ function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleNa result = tryResolve(extensions); } - return createResolvedModuleWithFailedLookupLocations( - compilerOptions, + return createResolvedModuleWithFailedLookupLocationsHandlingSymlink( + moduleName, result?.value?.resolved, result?.value?.isExternalLibraryImport, failedLookupLocations, affectingLocations, diagnostics, - state.resultFromCache + state, ); function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> { @@ -1823,18 +1863,8 @@ function nodeModuleNameResolverWorker(features: NodeResolutionFeatures, moduleNa } resolved = loadModuleFromNearestNodeModulesDirectory(extensions, moduleName, containingDirectory, state, cache, redirectedReference); } - if (!resolved) return undefined; - - let resolvedValue = resolved.value; - if (!compilerOptions.preserveSymlinks && resolvedValue && !resolvedValue.originalPath) { - const path = realPath(resolvedValue.path, host, traceEnabled); - const pathsAreEqual = arePathsEqual(path, resolvedValue.path, host); - const originalPath = pathsAreEqual ? undefined : resolvedValue.path; - // If the path and realpath are differing only in casing prefer path so that we can issue correct errors for casing under forceConsistentCasingInFileNames - resolvedValue = { ...resolvedValue, path: pathsAreEqual ? resolvedValue.path : path, originalPath }; - } // For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files. - return { value: resolvedValue && { resolved: resolvedValue, isExternalLibraryImport: true } }; + return resolved && { value: resolved.value && { resolved: resolved.value, isExternalLibraryImport: true } }; } else { const { path: candidate, parts } = normalizePathForCJSResolution(containingDirectory, moduleName); @@ -3061,14 +3091,14 @@ export function classicNameResolver(moduleName: string, containingFile: string, tryResolve(Extensions.TypeScript | Extensions.Declaration) || tryResolve(Extensions.JavaScript | (compilerOptions.resolveJsonModule ? Extensions.Json : 0)); // No originalPath because classic resolution doesn't resolve realPath - return createResolvedModuleWithFailedLookupLocations( - compilerOptions, + return createResolvedModuleWithFailedLookupLocationsHandlingSymlink( + moduleName, resolved && resolved.value, resolved?.value && pathContainsNodeModules(resolved.value.path), failedLookupLocations, affectingLocations, diagnostics, - state.resultFromCache + state, ); function tryResolve(extensions: Extensions): SearchResult { diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_withPaths.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_withPaths.trace.json index 6163a55512404..27af82f0086d9 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_withPaths.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_withPaths.trace.json @@ -11,6 +11,7 @@ "File '/node_modules/foo/lib/test.tsx' does not exist.", "File '/node_modules/foo/lib/test.d.ts' exist - use it as a name resolution result.", "File '/node_modules/foo/package.json' does not exist.", + "Resolving real path for '/node_modules/foo/lib/test.d.ts', result '/node_modules/foo/lib/test.d.ts'.", "======== Module name 'foo/test.js' was successfully resolved to '/node_modules/foo/lib/test.d.ts'. ========", "======== Resolving module 'foo/test' from '/test.ts'. ========", "Explicitly specified module resolution kind: 'NodeJs'.", @@ -23,6 +24,7 @@ "File '/node_modules/foo/lib/test.tsx' does not exist.", "File '/node_modules/foo/lib/test.d.ts' exist - use it as a name resolution result.", "File '/node_modules/foo/package.json' does not exist according to earlier cached lookups.", + "Resolving real path for '/node_modules/foo/lib/test.d.ts', result '/node_modules/foo/lib/test.d.ts'.", "======== Module name 'foo/test' was successfully resolved to '/node_modules/foo/lib/test.d.ts'. ========", "======== Resolving module './relative.js' from '/test.ts'. ========", "Explicitly specified module resolution kind: 'NodeJs'.", diff --git a/tests/baselines/reference/moduleResolutionWithSuffixes_one_externalModule_withPaths.trace.json b/tests/baselines/reference/moduleResolutionWithSuffixes_one_externalModule_withPaths.trace.json index 2e183a6dfe6a4..0d3998985e3a3 100644 --- a/tests/baselines/reference/moduleResolutionWithSuffixes_one_externalModule_withPaths.trace.json +++ b/tests/baselines/reference/moduleResolutionWithSuffixes_one_externalModule_withPaths.trace.json @@ -13,6 +13,7 @@ "File '/node_modules/some-library/lib/index.ios.ts' does not exist.", "File '/node_modules/some-library/lib/index.ios.tsx' does not exist.", "File '/node_modules/some-library/lib/index.ios.d.ts' exist - use it as a name resolution result.", + "Resolving real path for '/node_modules/some-library/lib/index.ios.d.ts', result '/node_modules/some-library/lib/index.ios.d.ts'.", "======== Module name 'some-library' was successfully resolved to '/node_modules/some-library/lib/index.ios.d.ts'. ========", "======== Resolving module 'some-library/index' from '/test.ts'. ========", "Explicitly specified module resolution kind: 'NodeJs'.", @@ -25,6 +26,7 @@ "File '/node_modules/some-library/lib/index.ios.tsx' does not exist.", "File '/node_modules/some-library/lib/index.ios.d.ts' exist - use it as a name resolution result.", "File '/node_modules/some-library/package.json' does not exist.", + "Resolving real path for '/node_modules/some-library/lib/index.ios.d.ts', result '/node_modules/some-library/lib/index.ios.d.ts'.", "======== Module name 'some-library/index' was successfully resolved to '/node_modules/some-library/lib/index.ios.d.ts'. ========", "======== Resolving module 'some-library/index.js' from '/test.ts'. ========", "Explicitly specified module resolution kind: 'NodeJs'.", @@ -38,5 +40,6 @@ "File '/node_modules/some-library/lib/index.ios.tsx' does not exist.", "File '/node_modules/some-library/lib/index.ios.d.ts' exist - use it as a name resolution result.", "File '/node_modules/some-library/package.json' does not exist according to earlier cached lookups.", + "Resolving real path for '/node_modules/some-library/lib/index.ios.d.ts', result '/node_modules/some-library/lib/index.ios.d.ts'.", "======== Module name 'some-library/index.js' was successfully resolved to '/node_modules/some-library/lib/index.ios.d.ts'. ========" ] \ No newline at end of file diff --git a/tests/baselines/reference/pathMappingBasedModuleResolution_withExtension_MapedToNodeModules.trace.json b/tests/baselines/reference/pathMappingBasedModuleResolution_withExtension_MapedToNodeModules.trace.json index ef53886c656c8..e670c4ef7ca0f 100644 --- a/tests/baselines/reference/pathMappingBasedModuleResolution_withExtension_MapedToNodeModules.trace.json +++ b/tests/baselines/reference/pathMappingBasedModuleResolution_withExtension_MapedToNodeModules.trace.json @@ -35,5 +35,6 @@ "File name '/node_modules/foo/bar/foobar.js' has a '.js' extension - stripping it.", "File '/node_modules/foo/bar/foobar.js' exist - use it as a name resolution result.", "File '/node_modules/foo/package.json' does not exist according to earlier cached lookups.", + "Resolving real path for '/node_modules/foo/bar/foobar.js', result '/node_modules/foo/bar/foobar.js'.", "======== Module name 'foo/bar/foobar.js' was successfully resolved to '/node_modules/foo/bar/foobar.js'. ========" ] \ No newline at end of file diff --git a/tests/baselines/reference/requireOfJsonFile_PathMapping.trace.json b/tests/baselines/reference/requireOfJsonFile_PathMapping.trace.json index 57579896df342..7bec1f8b165d2 100644 --- a/tests/baselines/reference/requireOfJsonFile_PathMapping.trace.json +++ b/tests/baselines/reference/requireOfJsonFile_PathMapping.trace.json @@ -26,5 +26,6 @@ "File name '/node_modules/foo/bar/foobar.json' has a '.json' extension - stripping it.", "File '/node_modules/foo/bar/foobar.json' exist - use it as a name resolution result.", "File '/node_modules/foo/package.json' does not exist according to earlier cached lookups.", + "Resolving real path for '/node_modules/foo/bar/foobar.json', result '/node_modules/foo/bar/foobar.json'.", "======== Module name 'foo/bar/foobar.json' was successfully resolved to '/node_modules/foo/bar/foobar.json'. ========" ] \ No newline at end of file diff --git a/tests/baselines/reference/scopedPackagesClassic.trace.json b/tests/baselines/reference/scopedPackagesClassic.trace.json index b28156d1c3392..d449bb6696438 100644 --- a/tests/baselines/reference/scopedPackagesClassic.trace.json +++ b/tests/baselines/reference/scopedPackagesClassic.trace.json @@ -5,5 +5,6 @@ "File '/node_modules/@types/see__saw/package.json' does not exist.", "File '/node_modules/@types/see__saw.d.ts' does not exist.", "File '/node_modules/@types/see__saw/index.d.ts' exist - use it as a name resolution result.", + "Resolving real path for '/node_modules/@types/see__saw/index.d.ts', result '/node_modules/@types/see__saw/index.d.ts'.", "======== Module name '@see/saw' was successfully resolved to '/node_modules/@types/see__saw/index.d.ts'. ========" ] \ No newline at end of file diff --git a/tests/baselines/reference/typingsLookupAmd.trace.json b/tests/baselines/reference/typingsLookupAmd.trace.json index 31cd72105f1dd..d63bcbf625e0c 100644 --- a/tests/baselines/reference/typingsLookupAmd.trace.json +++ b/tests/baselines/reference/typingsLookupAmd.trace.json @@ -14,6 +14,7 @@ "File '/x/node_modules/@types/b/package.json' does not exist.", "File '/x/node_modules/@types/b.d.ts' does not exist.", "File '/x/node_modules/@types/b/index.d.ts' exist - use it as a name resolution result.", + "Resolving real path for '/x/node_modules/@types/b/index.d.ts', result '/x/node_modules/@types/b/index.d.ts'.", "======== Module name 'b' was successfully resolved to '/x/node_modules/@types/b/index.d.ts'. ========", "======== Resolving module 'a' from '/x/node_modules/@types/b/index.d.ts'. ========", "Module resolution kind is not specified, using 'Classic'.", @@ -38,6 +39,7 @@ "File '/node_modules/@types/a/package.json' does not exist.", "File '/node_modules/@types/a.d.ts' does not exist.", "File '/node_modules/@types/a/index.d.ts' exist - use it as a name resolution result.", + "Resolving real path for '/node_modules/@types/a/index.d.ts', result '/node_modules/@types/a/index.d.ts'.", "======== Module name 'a' was successfully resolved to '/node_modules/@types/a/index.d.ts'. ========", "======== Resolving type reference directive 'a', containing file '/__inferred type names__.ts', root directory '/node_modules/@types'. ========", "Resolving with primary search path '/node_modules/@types'.",