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(compiler-cli): shorten resolved module name in fileNameToModuleName to npm package name for typings #23231

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions integration/bazel/BUILD.bazel
Expand Up @@ -4,9 +4,12 @@ filegroup(
name = "node_modules",
srcs = glob(
["node_modules/**/*"],
# Exclude directories that commonly contain filenames which are
# illegal bazel labels
exclude = [
# Exclude rxjs because we build it from sources using the label @rxjs//:rxjs
"node_modules/rxjs/**",

# Exclude directories that commonly contain filenames which are
# illegal bazel labels
# e.g. node_modules/adm-zip/test/assets/attributes_test/New folder/hidden.txt
"node_modules/**/test/**",
# e.g. node_modules/xpath/docs/function resolvers.md
Expand Down
33 changes: 29 additions & 4 deletions packages/compiler-cli/src/transformers/compiler_host.ts
Expand Up @@ -57,6 +57,7 @@ function assert<T>(condition: T | null | undefined) {
export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHost, AotCompilerHost,
TypeCheckHost {
private metadataReaderCache = createMetadataReaderCache();
private fileNameToModuleNameCache = new Map<string, string>();
private flatModuleIndexCache = new Map<string, boolean>();
private flatModuleIndexNames = new Set<string>();
private flatModuleIndexRedirectNames = new Set<string>();
Expand Down Expand Up @@ -187,6 +188,12 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos
* import project sources.
*/
fileNameToModuleName(importedFile: string, containingFile: string): string {
const cacheKey = `${importedFile}:${containingFile}`;
let moduleName = this.fileNameToModuleNameCache.get(cacheKey);
if (moduleName != null) {
return moduleName;
}

const originalImportedFile = importedFile;
if (this.options.traceResolution) {
console.error(
Expand All @@ -196,11 +203,10 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos

// drop extension
importedFile = importedFile.replace(EXT, '');
const importedFilePackagName = getPackageName(importedFile);
const importedFilePackageName = getPackageName(importedFile);
const containingFilePackageName = getPackageName(containingFile);

let moduleName: string;
if (importedFilePackagName === containingFilePackageName ||
if (importedFilePackageName === containingFilePackageName ||
GENERATED_FILES.test(originalImportedFile)) {
const rootedContainingFile = relativeToRootDirs(containingFile, this.rootDirs);
const rootedImportedFile = relativeToRootDirs(importedFile, this.rootDirs);
Expand All @@ -211,12 +217,31 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos
importedFile = rootedImportedFile;
}
moduleName = dotRelative(path.dirname(containingFile), importedFile);
} else if (importedFilePackagName) {
} else if (importedFilePackageName) {
moduleName = stripNodeModulesPrefix(importedFile);
if (originalImportedFile.endsWith('.d.ts')) {
// the moduleName for these typings could be shortented to the npm package name
// if the npm package typings matches the importedFile
try {
const modulePath = importedFile.substring(0, importedFile.length - moduleName.length) +
importedFilePackageName;
const packageJson = require(modulePath + '/package.json');
const packageTypings = path.posix.join(modulePath, packageJson.typings);
if (packageTypings === originalImportedFile) {
moduleName = importedFilePackageName;
}
} catch (e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a comment explaining why ignoring the error is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// the above require() will throw if there is no package.json file
// and this is safe to ignore and correct to keep the longer
// moduleName in this case
}
}
} else {
throw new Error(
`Trying to import a source file from a node_modules package: import ${originalImportedFile} from ${containingFile}`);
}

this.fileNameToModuleNameCache.set(cacheKey, moduleName);
return moduleName;
}

Expand Down