Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix declaration emit when the packages are included through symlinks #37438

Merged
merged 6 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4066,9 +4066,10 @@ namespace ts {
tracker: tracker && tracker.trackSymbol ? tracker : { trackSymbol: noop, moduleResolverHost: flags! & NodeBuilderFlags.DoNotIncludeSymbolChain ? {
getCommonSourceDirectory: (host as Program).getCommonSourceDirectory ? () => (host as Program).getCommonSourceDirectory() : () => "",
getSourceFiles: () => host.getSourceFiles(),
getCurrentDirectory: maybeBind(host, host.getCurrentDirectory),
getCurrentDirectory: () => host.getCurrentDirectory(),
getProbableSymlinks: maybeBind(host, host.getProbableSymlinks),
useCaseSensitiveFileNames: maybeBind(host, host.useCaseSensitiveFileNames)
useCaseSensitiveFileNames: maybeBind(host, host.useCaseSensitiveFileNames),
redirectTargetsMap: host.redirectTargetsMap,
} : undefined },
encounteredError: false,
visitedTypes: undefined,
Expand Down Expand Up @@ -5028,9 +5029,7 @@ namespace ts {
specifierCompilerOptions,
contextFile,
moduleResolverHost,
host.getSourceFiles(),
{ importModuleSpecifierPreference: isBundle ? "non-relative" : "relative" },
host.redirectTargetsMap,
));
links.specifierCache = links.specifierCache || createMap();
links.specifierCache.set(contextFile.path, specifier);
Expand Down Expand Up @@ -6450,7 +6449,7 @@ namespace ts {
const getCanonicalFileName = createGetCanonicalFileName(!!host.useCaseSensitiveFileNames);
const resolverHost = {
getCanonicalFileName,
getCurrentDirectory: context.tracker.moduleResolverHost.getCurrentDirectory ? () => context.tracker.moduleResolverHost!.getCurrentDirectory!() : () => "",
getCurrentDirectory: () => context.tracker.moduleResolverHost!.getCurrentDirectory(),
getCommonSourceDirectory: () => context.tracker.moduleResolverHost!.getCommonSourceDirectory()
};
const newName = getResolvedExternalModuleName(resolverHost, targetFile);
Expand Down
1 change: 0 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,6 @@ namespace ts {
isEmitBlocked: returnFalse,
readFile: f => host.readFile(f),
fileExists: f => host.fileExists(f),
directoryExists: host.directoryExists && (f => host.directoryExists!(f)),
useCaseSensitiveFileNames: () => host.useCaseSensitiveFileNames(),
getProgramBuildInfo: returnUndefined,
getSourceFileFromReference: returnUndefined,
Expand Down
49 changes: 20 additions & 29 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,9 @@ namespace ts.moduleSpecifiers {
importingSourceFileName: Path,
toFileName: string,
host: ModuleSpecifierResolutionHost,
files: readonly SourceFile[],
redirectTargetsMap: RedirectTargetsMap,
oldImportSpecifier: string,
): string | undefined {
const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferencesForUpdate(compilerOptions, oldImportSpecifier));
const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferencesForUpdate(compilerOptions, oldImportSpecifier));
if (res === oldImportSpecifier) return undefined;
return res;
}
Expand All @@ -57,23 +55,19 @@ namespace ts.moduleSpecifiers {
importingSourceFileName: Path,
toFileName: string,
host: ModuleSpecifierResolutionHost,
files: readonly SourceFile[],
preferences: UserPreferences = {},
redirectTargetsMap: RedirectTargetsMap,
): string {
return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferences(preferences, compilerOptions, importingSourceFile));
return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, getPreferences(preferences, compilerOptions, importingSourceFile));
}

export function getNodeModulesPackageName(
compilerOptions: CompilerOptions,
importingSourceFileName: Path,
nodeModulesFileName: string,
host: ModuleSpecifierResolutionHost,
files: readonly SourceFile[],
redirectTargetsMap: RedirectTargetsMap,
): string | undefined {
const info = getInfo(importingSourceFileName, host);
const modulePaths = getAllModulePaths(files, importingSourceFileName, nodeModulesFileName, info.getCanonicalFileName, host, redirectTargetsMap);
const modulePaths = getAllModulePaths(importingSourceFileName, nodeModulesFileName, host);
return firstDefined(modulePaths,
moduleFileName => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions, /*packageNameOnly*/ true));
}
Expand All @@ -83,12 +77,10 @@ namespace ts.moduleSpecifiers {
importingSourceFileName: Path,
toFileName: string,
host: ModuleSpecifierResolutionHost,
files: readonly SourceFile[],
redirectTargetsMap: RedirectTargetsMap,
preferences: Preferences
): string {
const info = getInfo(importingSourceFileName, host);
const modulePaths = getAllModulePaths(files, importingSourceFileName, toFileName, info.getCanonicalFileName, host, redirectTargetsMap);
const modulePaths = getAllModulePaths(importingSourceFileName, toFileName, host);
return firstDefined(modulePaths, moduleFileName => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions)) ||
getLocalModuleSpecifier(toFileName, info, compilerOptions, preferences);
}
Expand All @@ -99,16 +91,14 @@ namespace ts.moduleSpecifiers {
compilerOptions: CompilerOptions,
importingSourceFile: SourceFile,
host: ModuleSpecifierResolutionHost,
files: readonly SourceFile[],
userPreferences: UserPreferences,
redirectTargetsMap: RedirectTargetsMap,
): readonly string[] {
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
if (ambient) return [ambient];

const info = getInfo(importingSourceFile.path, host);
const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration || getNonAugmentationDeclaration(moduleSymbol));
const modulePaths = getAllModulePaths(files, importingSourceFile.path, moduleSourceFile.originalFileName, info.getCanonicalFileName, host, redirectTargetsMap);
const modulePaths = getAllModulePaths(importingSourceFile.path, moduleSourceFile.originalFileName, host);

const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);
const global = mapDefined(modulePaths, moduleFileName => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions));
Expand Down Expand Up @@ -179,26 +169,24 @@ namespace ts.moduleSpecifiers {
}

export function forEachFileNameOfModule<T>(
files: readonly SourceFile[],
importingFileName: string,
importedFileName: string,
getCanonicalFileName: GetCanonicalFileName,
host: ModuleSpecifierResolutionHost,
redirectTargetsMap: RedirectTargetsMap,
preferSymlinks: boolean,
cb: (fileName: string) => T | undefined
): T | undefined {
const redirects = redirectTargetsMap.get(importedFileName);
const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName];
const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
const getCanonicalFileName = hostGetCanonicalFileName(host);
const cwd = host.getCurrentDirectory();
const redirects = host.redirectTargetsMap.get(toPath(importedFileName, cwd, getCanonicalFileName)) || emptyArray;
const importedFileNames = [importedFileName, ...redirects];
sheetalkamat marked this conversation as resolved.
Show resolved Hide resolved
const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd));
if (!preferSymlinks) {
const result = forEach(targets, cb);
if (result) return result;
}
const links = host.getProbableSymlinks
? host.getProbableSymlinks(files)
: discoverProbableSymlinks(files, getCanonicalFileName, cwd);
? host.getProbableSymlinks(host.getSourceFiles())
: discoverProbableSymlinks(host.getSourceFiles(), getCanonicalFileName, cwd);

const compareStrings = (!host.useCaseSensitiveFileNames || host.useCaseSensitiveFileNames()) ? compareStringsCaseSensitive : compareStringsCaseInsensitive;
const result = forEachEntry(links, (resolved, path) => {
Expand All @@ -224,20 +212,20 @@ 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: readonly SourceFile[], importingFileName: string, importedFileName: string, getCanonicalFileName: GetCanonicalFileName, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): readonly string[] {
const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
function getAllModulePaths(importingFileName: string, importedFileName: string, host: ModuleSpecifierResolutionHost): readonly string[] {
const cwd = host.getCurrentDirectory();
const getCanonicalFileName = hostGetCanonicalFileName(host);
const allFileNames = createMap<string>();
let importedFileFromNodeModules = false;
forEachFileNameOfModule(
files,
importingFileName,
importedFileName,
getCanonicalFileName,
host,
redirectTargetsMap,
/*preferSymlinks*/ true,
path => {
// dont return value, so we collect everything
allFileNames.set(path, getCanonicalFileName(path));
importedFileFromNodeModules = importedFileFromNodeModules || pathContainsNodeModules(path);
}
);

Expand All @@ -251,7 +239,10 @@ namespace ts.moduleSpecifiers {
let pathsInDirectory: string[] | undefined;
allFileNames.forEach((canonicalFileName, fileName) => {
if (startsWith(canonicalFileName, directoryStart)) {
(pathsInDirectory || (pathsInDirectory = [])).push(fileName);
// If the importedFile is from node modules, use only paths in node_modules folder as option
if (!importedFileFromNodeModules || pathContainsNodeModules(fileName)) {
(pathsInDirectory || (pathsInDirectory = [])).push(fileName);
}
sheetalkamat marked this conversation as resolved.
Show resolved Hide resolved
allFileNames.delete(fileName);
}
});
Expand Down
1 change: 0 additions & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,6 @@ namespace ts {
// Before falling back to the host
return host.fileExists(f);
},
...(host.directoryExists ? { directoryExists: f => host.directoryExists!(f) } : {}),
useCaseSensitiveFileNames: () => host.useCaseSensitiveFileNames(),
getProgramBuildInfo: () => program.getProgramBuildInfo && program.getProgramBuildInfo(),
getSourceFileFromReference: (file, ref) => program.getSourceFileFromReference(file, ref),
Expand Down
4 changes: 1 addition & 3 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,7 @@ namespace ts {
toPath(outputFilePath, host.getCurrentDirectory(), host.getCanonicalFileName),
toPath(declFileName, host.getCurrentDirectory(), host.getCanonicalFileName),
host,
host.getSourceFiles(),
/*preferences*/ undefined,
host.redirectTargetsMap
);
if (!pathIsRelative(specifier)) {
// If some compiler option/symlink/whatever allows access to the file containing the ambient module declaration
Expand All @@ -376,7 +374,7 @@ namespace ts {

// omit references to files from node_modules (npm may disambiguate module
// references when installing this package, making the path is unreliable).
if (startsWith(fileName, "node_modules/") || fileName.indexOf("/node_modules/") !== -1) {
if (startsWith(fileName, "node_modules/") || pathContainsNodeModules(fileName)) {
return;
}

Expand Down
16 changes: 12 additions & 4 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3187,8 +3187,7 @@ namespace ts {
file: Path;
}

// TODO: This should implement TypeCheckerHost but that's an internal type.
export interface Program extends ScriptReferenceHost, ModuleSpecifierResolutionHost {
export interface Program extends ScriptReferenceHost {
getCurrentDirectory(): string;
/**
* Get a list of root file names that were passed to a 'createProgram'
Expand Down Expand Up @@ -3290,6 +3289,10 @@ namespace ts {
/*@internal*/ getProbableSymlinks(): ReadonlyMap<string>;
}

/*@internal*/
export interface Program extends TypeCheckerHost, ModuleSpecifierResolutionHost {
}

/* @internal */
export type RedirectTargetsMap = ReadonlyMap<readonly string[]>;

Expand Down Expand Up @@ -6420,14 +6423,19 @@ namespace ts {
getCurrentDirectory?(): string;
}

export interface ModuleSpecifierResolutionHost extends GetEffectiveTypeRootsHost {
/*@internal*/
export interface ModuleSpecifierResolutionHost {
useCaseSensitiveFileNames?(): boolean;
fileExists?(path: string): boolean;
getCurrentDirectory(): string;
readFile?(path: string): string | undefined;
/* @internal */
getProbableSymlinks?(files: readonly SourceFile[]): ReadonlyMap<string>;
/* @internal */
getGlobalTypingsCacheLocation?(): string | undefined;

getSourceFiles(): readonly SourceFile[];
readonly redirectTargetsMap: RedirectTargetsMap;
}

// Note: this used to be deprecated in our public API, but is still used internally
Expand All @@ -6441,7 +6449,7 @@ namespace ts {
reportPrivateInBaseOfClassExpression?(propertyName: string): void;
reportInaccessibleUniqueSymbolError?(): void;
reportLikelyUnsafeImportRequiredError?(specifier: string): void;
moduleResolverHost?: ModuleSpecifierResolutionHost & { getSourceFiles(): readonly SourceFile[], getCommonSourceDirectory(): string };
moduleResolverHost?: ModuleSpecifierResolutionHost & { getCommonSourceDirectory(): string };
trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;
trackExternalModuleSymbolOfImportTypeNode?(symbol: Symbol): void;
}
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3753,6 +3753,14 @@ namespace ts {
};
}

export function hostUsesCaseSensitiveFileNames(host: { useCaseSensitiveFileNames?(): boolean; }): boolean {
return host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false;
}

export function hostGetCanonicalFileName(host: { useCaseSensitiveFileNames?(): boolean; }): GetCanonicalFileName {
return createGetCanonicalFileName(hostUsesCaseSensitiveFileNames(host));
}

export interface ResolveModuleNameResolutionHost {
getCanonicalFileName(p: string): string;
getCommonSourceDirectory(): string;
Expand Down
4 changes: 2 additions & 2 deletions src/harness/virtualFileSystemWithWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ interface Array<T> { length: number; [n: number]: T; }`
}

export type FileOrFolderOrSymLink = File | Folder | SymLink;
function isFile(fileOrFolderOrSymLink: FileOrFolderOrSymLink): fileOrFolderOrSymLink is File {
export function isFile(fileOrFolderOrSymLink: FileOrFolderOrSymLink): fileOrFolderOrSymLink is File {
return isString((<File>fileOrFolderOrSymLink).content);
}

function isSymLink(fileOrFolderOrSymLink: FileOrFolderOrSymLink): fileOrFolderOrSymLink is SymLink {
export function isSymLink(fileOrFolderOrSymLink: FileOrFolderOrSymLink): fileOrFolderOrSymLink is SymLink {
return isString((<SymLink>fileOrFolderOrSymLink).symLink);
}

Expand Down
14 changes: 1 addition & 13 deletions src/services/codefixes/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,10 @@ namespace ts.codefix {
}
}

function getModuleSpecifierResolverHost(context: TypeConstructionContext): SymbolTracker["moduleResolverHost"] {
return {
directoryExists: context.host.directoryExists ? d => context.host.directoryExists!(d) : undefined,
fileExists: context.host.fileExists ? f => context.host.fileExists!(f) : undefined,
getCurrentDirectory: context.host.getCurrentDirectory ? () => context.host.getCurrentDirectory() : undefined,
readFile: context.host.readFile ? f => context.host.readFile!(f) : undefined,
useCaseSensitiveFileNames: context.host.useCaseSensitiveFileNames ? () => context.host.useCaseSensitiveFileNames!() : undefined,
getSourceFiles: () => context.program.getSourceFiles(),
getCommonSourceDirectory: () => context.program.getCommonSourceDirectory(),
};
}

export function getNoopSymbolTrackerWithResolver(context: TypeConstructionContext): SymbolTracker {
return {
trackSymbol: noop,
moduleResolverHost: getModuleSpecifierResolverHost(context),
moduleResolverHost: getModuleSpecifierResolverHost(context.program, context.host),
};
}

Expand Down
Loading