Skip to content
Permalink
Browse files

fix(ivy): ngcc should only index .d.ts exports within the package (#3…

…2129)

ngcc needs to solve a unique problem when compiling typings for an
entrypoint: it must resolve a declaration within a .js file to its
representation in a .d.ts file. Since such .d.ts files can be used in deep
imports without ever being referenced from the "root" .d.ts, it's not enough
to simply match exported types to the root .d.ts. ngcc must build an index
of all .d.ts files.

Previously, this operation had a bug: it scanned all .d.ts files in the
.d.ts program, not only those within the package. Thus, if a class in the
program happened to share a name with a class exported from a dependency's
.d.ts, ngcc might accidentally modify the wrong .d.ts file, causing a
variety of issues downstream.

To fix this issue, ngcc's .d.ts scanner now limits the .d.ts files it
indexes to only those declared in the current package.

PR Close #32129
  • Loading branch information...
alxhub authored and AndrewKushnir committed Aug 14, 2019
1 parent 02bab8c commit 964d72610f766e7900a238b8de49ed86be9fd6f6
@@ -10,6 +10,7 @@ import * as ts from 'typescript';

import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, ConcreteDeclaration, CtorParameter, Declaration, Decorator, TypeScriptReflectionHost, isDecoratorIdentifier, reflectObjectLiteral} from '../../../src/ngtsc/reflection';
import {isWithinPackage} from '../analysis/util';
import {Logger} from '../logging/logger';
import {BundleProgram} from '../packages/bundle_program';
import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils';
@@ -85,7 +86,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
protected logger: Logger, protected isCore: boolean, checker: ts.TypeChecker,
dts?: BundleProgram|null) {
super(checker);
this.dtsDeclarationMap = dts && this.computeDtsDeclarationMap(dts.path, dts.program) || null;
this.dtsDeclarationMap =
dts && this.computeDtsDeclarationMap(dts.path, dts.program, dts.package) || null;
}

/**
@@ -1347,8 +1349,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
* @param dtsProgram The program containing all the typings files.
* @returns a map of class names to class declarations.
*/
protected computeDtsDeclarationMap(dtsRootFileName: AbsoluteFsPath, dtsProgram: ts.Program):
Map<string, ts.Declaration> {
protected computeDtsDeclarationMap(
dtsRootFileName: AbsoluteFsPath, dtsProgram: ts.Program,
dtsPackage: AbsoluteFsPath): Map<string, ts.Declaration> {
const dtsDeclarationMap = new Map<string, ts.Declaration>();
const checker = dtsProgram.getTypeChecker();

@@ -1361,8 +1364,12 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N

// Now add any additional classes that are exported from individual dts files,
// but are not publicly exported from the entry-point.
dtsProgram.getSourceFiles().forEach(
sourceFile => { collectExportedDeclarations(checker, dtsDeclarationMap, sourceFile); });
dtsProgram.getSourceFiles().forEach(sourceFile => {
if (!isWithinPackage(dtsPackage, sourceFile)) {
return;
}
collectExportedDeclarations(checker, dtsDeclarationMap, sourceFile);
});
return dtsDeclarationMap;
}

@@ -23,6 +23,7 @@ export interface BundleProgram {
host: ts.CompilerHost;
path: AbsoluteFsPath;
file: ts.SourceFile;
package: AbsoluteFsPath;
r3SymbolsPath: AbsoluteFsPath|null;
r3SymbolsFile: ts.SourceFile|null;
}
@@ -31,7 +32,7 @@ export interface BundleProgram {
* Create a bundle program.
*/
export function makeBundleProgram(
fs: FileSystem, isCore: boolean, path: AbsoluteFsPath, r3FileName: string,
fs: FileSystem, isCore: boolean, pkg: AbsoluteFsPath, path: AbsoluteFsPath, r3FileName: string,
options: ts.CompilerOptions, host: ts.CompilerHost,
additionalFiles: AbsoluteFsPath[] = []): BundleProgram {
const r3SymbolsPath = isCore ? findR3SymbolsPath(fs, dirname(path), r3FileName) : null;
@@ -48,7 +49,7 @@ export function makeBundleProgram(
const file = program.getSourceFile(path) !;
const r3SymbolsFile = r3SymbolsPath && program.getSourceFile(r3SymbolsPath) || null;

return {program, options, host, path, file, r3SymbolsPath, r3SymbolsFile};
return {program, options, host, package: pkg, path, file, r3SymbolsPath, r3SymbolsFile};
}

/**
@@ -57,14 +57,15 @@ export function makeEntryPointBundle(
// Create the bundle programs, as necessary.
const absFormatPath = fs.resolve(entryPoint.path, formatPath);
const typingsPath = fs.resolve(entryPoint.path, entryPoint.typings);
const src = makeBundleProgram(fs, isCore, absFormatPath, 'r3_symbols.js', options, srcHost);
const src = makeBundleProgram(
fs, isCore, entryPoint.package, absFormatPath, 'r3_symbols.js', options, srcHost);
const additionalDtsFiles = transformDts && mirrorDtsFromSrc ?
computePotentialDtsFilesFromJsFiles(fs, src.program, absFormatPath, typingsPath) :
[];
const dts = transformDts ?
makeBundleProgram(
fs, isCore, typingsPath, 'r3_symbols.d.ts', options, dtsHost, additionalDtsFiles) :
null;
const dts = transformDts ? makeBundleProgram(
fs, isCore, entryPoint.package, typingsPath, 'r3_symbols.d.ts',
options, dtsHost, additionalDtsFiles) :
null;
const isFlatCore = isCore && src.r3SymbolsFile === null;

return {entryPoint, format, rootDirs, isCore, isFlatCore, src, dts};
@@ -36,28 +36,31 @@ export function makeTestEntryPointBundle(
dtsRootNames?: AbsoluteFsPath[]): EntryPointBundle {
const entryPoint = makeTestEntryPoint(packageName);
const src = makeTestBundleProgram(srcRootNames[0], isCore);
const dts = dtsRootNames ? makeTestDtsBundleProgram(dtsRootNames[0], isCore) : null;
const dts =
dtsRootNames ? makeTestDtsBundleProgram(dtsRootNames[0], entryPoint.package, isCore) : null;
const isFlatCore = isCore && src.r3SymbolsFile === null;
return {entryPoint, format, rootDirs: [absoluteFrom('/')], src, dts, isCore, isFlatCore};
}

export function makeTestBundleProgram(
path: AbsoluteFsPath, isCore: boolean = false): BundleProgram {
path: AbsoluteFsPath, isCore: boolean = false,
additionalFiles?: AbsoluteFsPath[]): BundleProgram {
const fs = getFileSystem();
const entryPointPath = fs.dirname(path);
const rootDir = fs.dirname(entryPointPath);
const options: ts.CompilerOptions =
{allowJs: true, maxNodeModuleJsDepth: Infinity, checkJs: false, rootDir, rootDirs: [rootDir]};
const host = new NgccSourcesCompilerHost(fs, options, entryPointPath);
return makeBundleProgram(fs, isCore, path, 'r3_symbols.js', options, host);
return makeBundleProgram(
fs, isCore, rootDir, path, 'r3_symbols.js', options, host, additionalFiles);
}

export function makeTestDtsBundleProgram(
path: AbsoluteFsPath, isCore: boolean = false): BundleProgram {
path: AbsoluteFsPath, packagePath: AbsoluteFsPath, isCore: boolean = false): BundleProgram {
const fs = getFileSystem();
const options = {};
const host = new NgtscCompilerHost(fs, options);
return makeBundleProgram(fs, isCore, path, 'r3_symbols.d.ts', options, host);
return makeBundleProgram(fs, isCore, packagePath, path, 'r3_symbols.d.ts', options, host);
}

export function convertToDirectTsLibImport(filesystem: TestFile[]) {
@@ -1992,7 +1992,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const class1 =
getDeclaration(program, _('/src/class1.js'), 'Class1', ts.isVariableDeclaration);
const host =
@@ -2006,7 +2006,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const mooFn =
getDeclaration(program, _('/src/func1.js'), 'mooFn', ts.isFunctionDeclaration);
const host =
@@ -2019,7 +2019,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const missingClass = getDeclaration(
program, _('/src/class1.js'), 'MissingClass1', ts.isVariableDeclaration);
const host =
@@ -2032,7 +2032,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const missingClass = getDeclaration(
program, _('/src/missing-class.js'), 'MissingClass2', ts.isVariableDeclaration);
const host =
@@ -2046,7 +2046,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const class1 = getDeclaration(
program, _('/src/flat-file.js'), 'Class1', ts.isVariableDeclaration);
const host =
@@ -2060,7 +2060,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const class3 =
getDeclaration(program, _('/src/flat-file.js'), 'Class3', ts.isVariableDeclaration);
const host =
@@ -2075,7 +2075,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const internalClass = getDeclaration(
program, _('/src/internal.js'), 'InternalClass', ts.isVariableDeclaration);
const host =
@@ -2090,7 +2090,7 @@ exports.ExternalModule = ExternalModule;
loadTestFiles(TYPINGS_SRC_FILES);
loadTestFiles(TYPINGS_DTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/src/index.js'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'));
const dts = makeTestDtsBundleProgram(_('/typings/index.d.ts'), _('/'));
const class2 =
getDeclaration(program, _('/src/class2.js'), 'Class2', ts.isVariableDeclaration);
const internalClass2 =

0 comments on commit 964d726

Please sign in to comment.
You can’t perform that action at this time.