fix(compiler): use either summary or metadata information when reading .d.ts files (#18912)

This fix applies for getting all symbols in file.

PR Close #18912
This commit is contained in:
Tobias Bosch 2017-08-28 15:51:27 -07:00 committed by Jason Aden
parent 83e5deb988
commit f1e526f046
4 changed files with 47 additions and 39 deletions

View File

@ -269,17 +269,20 @@ export class StaticSymbolResolver {
} }
getSymbolsOf(filePath: string): StaticSymbol[] { getSymbolsOf(filePath: string): StaticSymbol[] {
const summarySymbols = this.summaryResolver.getSymbolsOf(filePath);
if (summarySymbols) {
return summarySymbols;
}
// Note: Some users use libraries that were not compiled with ngc, i.e. they don't // Note: Some users use libraries that were not compiled with ngc, i.e. they don't
// have summaries, only .d.ts files. So we always need to check both, the summary // have summaries, only .d.ts files, but `summaryResolver.isLibraryFile` returns true.
// and metadata.
let symbols = new Set<StaticSymbol>(this.summaryResolver.getSymbolsOf(filePath));
this._createSymbolsOf(filePath); this._createSymbolsOf(filePath);
const metadataSymbols: StaticSymbol[] = [];
this.resolvedSymbols.forEach((resolvedSymbol) => { this.resolvedSymbols.forEach((resolvedSymbol) => {
if (resolvedSymbol.symbol.filePath === filePath) { if (resolvedSymbol.symbol.filePath === filePath) {
symbols.add(resolvedSymbol.symbol); metadataSymbols.push(resolvedSymbol.symbol);
} }
}); });
return Array.from(symbols); return metadataSymbols;
} }
private _createSymbolsOf(filePath: string) { private _createSymbolsOf(filePath: string) {

View File

@ -42,7 +42,7 @@ export interface AotSummaryResolverHost {
export class AotSummaryResolver implements SummaryResolver<StaticSymbol> { export class AotSummaryResolver implements SummaryResolver<StaticSymbol> {
// Note: this will only contain StaticSymbols without members! // Note: this will only contain StaticSymbols without members!
private summaryCache = new Map<StaticSymbol, Summary<StaticSymbol>>(); private summaryCache = new Map<StaticSymbol, Summary<StaticSymbol>>();
private loadedFilePaths = new Set<string>(); private loadedFilePaths = new Map<string, boolean>();
// Note: this will only contain StaticSymbols without members! // Note: this will only contain StaticSymbols without members!
private importAs = new Map<StaticSymbol, StaticSymbol>(); private importAs = new Map<StaticSymbol, StaticSymbol>();
@ -63,19 +63,21 @@ export class AotSummaryResolver implements SummaryResolver<StaticSymbol> {
return this.host.fromSummaryFileName(fileName, referringLibFileName); return this.host.fromSummaryFileName(fileName, referringLibFileName);
} }
resolveSummary(staticSymbol: StaticSymbol): Summary<StaticSymbol> { resolveSummary(staticSymbol: StaticSymbol): Summary<StaticSymbol>|null {
staticSymbol.assertNoMembers(); staticSymbol.assertNoMembers();
let summary = this.summaryCache.get(staticSymbol); let summary = this.summaryCache.get(staticSymbol);
if (!summary) { if (!summary) {
this._loadSummaryFile(staticSymbol.filePath); this._loadSummaryFile(staticSymbol.filePath);
summary = this.summaryCache.get(staticSymbol) !; summary = this.summaryCache.get(staticSymbol) !;
} }
return summary; return summary || null;
} }
getSymbolsOf(filePath: string): StaticSymbol[] { getSymbolsOf(filePath: string): StaticSymbol[]|null {
this._loadSummaryFile(filePath); if (this._loadSummaryFile(filePath)) {
return Array.from(this.summaryCache.keys()).filter((symbol) => symbol.filePath === filePath); return Array.from(this.summaryCache.keys()).filter((symbol) => symbol.filePath === filePath);
}
return null;
} }
getImportAs(staticSymbol: StaticSymbol): StaticSymbol { getImportAs(staticSymbol: StaticSymbol): StaticSymbol {
@ -85,30 +87,33 @@ export class AotSummaryResolver implements SummaryResolver<StaticSymbol> {
addSummary(summary: Summary<StaticSymbol>) { this.summaryCache.set(summary.symbol, summary); } addSummary(summary: Summary<StaticSymbol>) { this.summaryCache.set(summary.symbol, summary); }
private _loadSummaryFile(filePath: string) { private _loadSummaryFile(filePath: string): boolean {
if (this.loadedFilePaths.has(filePath)) { let hasSummary = this.loadedFilePaths.get(filePath);
return; if (hasSummary != null) {
return hasSummary;
} }
this.loadedFilePaths.add(filePath); let json: string|null = null;
if (this.isLibraryFile(filePath)) { if (this.isLibraryFile(filePath)) {
const summaryFilePath = summaryFileName(filePath); const summaryFilePath = summaryFileName(filePath);
let json: string|null;
try { try {
json = this.host.loadSummary(summaryFilePath); json = this.host.loadSummary(summaryFilePath);
} catch (e) { } catch (e) {
console.error(`Error loading summary file ${summaryFilePath}`); console.error(`Error loading summary file ${summaryFilePath}`);
throw e; throw e;
} }
if (json) {
const {summaries, importAs} =
deserializeSummaries(this.staticSymbolCache, this, filePath, json);
summaries.forEach((summary) => this.summaryCache.set(summary.symbol, summary));
importAs.forEach((importAs) => {
this.importAs.set(
importAs.symbol,
this.staticSymbolCache.get(ngfactoryFilePath(filePath), importAs.importAs));
});
}
} }
hasSummary = json != null;
this.loadedFilePaths.set(filePath, hasSummary);
if (json) {
const {summaries, importAs} =
deserializeSummaries(this.staticSymbolCache, this, filePath, json);
summaries.forEach((summary) => this.summaryCache.set(summary.symbol, summary));
importAs.forEach((importAs) => {
this.importAs.set(
importAs.symbol,
this.staticSymbolCache.get(ngfactoryFilePath(filePath), importAs.importAs));
});
}
return hasSummary;
} }
} }

View File

@ -19,7 +19,7 @@ export abstract class SummaryResolver<T> {
abstract toSummaryFileName(fileName: string, referringSrcFileName: string): string; abstract toSummaryFileName(fileName: string, referringSrcFileName: string): string;
abstract fromSummaryFileName(fileName: string, referringLibFileName: string): string; abstract fromSummaryFileName(fileName: string, referringLibFileName: string): string;
abstract resolveSummary(reference: T): Summary<T>|null; abstract resolveSummary(reference: T): Summary<T>|null;
abstract getSymbolsOf(filePath: string): T[]; abstract getSymbolsOf(filePath: string): T[]|null;
abstract getImportAs(reference: T): T; abstract getImportAs(reference: T): T;
abstract addSummary(summary: Summary<T>): void; abstract addSummary(summary: Summary<T>): void;
} }

View File

@ -136,15 +136,14 @@ describe('StaticSymbolResolver', () => {
]); ]);
}); });
it('should merge the exported symbols of a file with the exported symbols of its summary', () => { it('should read the exported symbols of a file from the summary and ignore exports in the source',
const someSymbol = symbolCache.get('/test.ts', 'a'); () => {
init( const someSymbol = symbolCache.get('/test.ts', 'a');
{'/test.ts': 'export var b = 2'}, init(
[{symbol: symbolCache.get('/test.ts', 'a'), metadata: 1}]); {'/test.ts': 'export var b = 2'},
expect(symbolResolver.getSymbolsOf('/test.ts')).toEqual([ [{symbol: symbolCache.get('/test.ts', 'a'), metadata: 1}]);
symbolCache.get('/test.ts', 'a'), symbolCache.get('/test.ts', 'b') expect(symbolResolver.getSymbolsOf('/test.ts')).toEqual([symbolCache.get('/test.ts', 'a')]);
]); });
});
describe('importAs', () => { describe('importAs', () => {
@ -385,9 +384,10 @@ export class MockSummaryResolver implements SummaryResolver<StaticSymbol> {
resolveSummary(reference: StaticSymbol): Summary<StaticSymbol> { resolveSummary(reference: StaticSymbol): Summary<StaticSymbol> {
return this.summaries.find(summary => summary.symbol === reference); return this.summaries.find(summary => summary.symbol === reference);
}; };
getSymbolsOf(filePath: string): StaticSymbol[] { getSymbolsOf(filePath: string): StaticSymbol[]|null {
return this.summaries.filter(summary => summary.symbol.filePath === filePath) const symbols = this.summaries.filter(summary => summary.symbol.filePath === filePath)
.map(summary => summary.symbol); .map(summary => summary.symbol);
return symbols.length ? symbols : null;
} }
getImportAs(symbol: StaticSymbol): StaticSymbol { getImportAs(symbol: StaticSymbol): StaticSymbol {
const entry = this.importAs.find(entry => entry.symbol === symbol); const entry = this.importAs.find(entry => entry.symbol === symbol);