From f1e526f046a78abf970a2a24e274a6d718967954 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 28 Aug 2017 15:51:27 -0700 Subject: [PATCH] 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 --- .../src/aot/static_symbol_resolver.ts | 13 +++-- packages/compiler/src/aot/summary_resolver.ts | 47 ++++++++++--------- packages/compiler/src/summary_resolver.ts | 2 +- .../test/aot/static_symbol_resolver_spec.ts | 24 +++++----- 4 files changed, 47 insertions(+), 39 deletions(-) diff --git a/packages/compiler/src/aot/static_symbol_resolver.ts b/packages/compiler/src/aot/static_symbol_resolver.ts index b3a1696b65..20e3dcf3a7 100644 --- a/packages/compiler/src/aot/static_symbol_resolver.ts +++ b/packages/compiler/src/aot/static_symbol_resolver.ts @@ -269,17 +269,20 @@ export class StaticSymbolResolver { } 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 - // have summaries, only .d.ts files. So we always need to check both, the summary - // and metadata. - let symbols = new Set(this.summaryResolver.getSymbolsOf(filePath)); + // have summaries, only .d.ts files, but `summaryResolver.isLibraryFile` returns true. this._createSymbolsOf(filePath); + const metadataSymbols: StaticSymbol[] = []; this.resolvedSymbols.forEach((resolvedSymbol) => { if (resolvedSymbol.symbol.filePath === filePath) { - symbols.add(resolvedSymbol.symbol); + metadataSymbols.push(resolvedSymbol.symbol); } }); - return Array.from(symbols); + return metadataSymbols; } private _createSymbolsOf(filePath: string) { diff --git a/packages/compiler/src/aot/summary_resolver.ts b/packages/compiler/src/aot/summary_resolver.ts index c630675dc7..4999dd231b 100644 --- a/packages/compiler/src/aot/summary_resolver.ts +++ b/packages/compiler/src/aot/summary_resolver.ts @@ -42,7 +42,7 @@ export interface AotSummaryResolverHost { export class AotSummaryResolver implements SummaryResolver { // Note: this will only contain StaticSymbols without members! private summaryCache = new Map>(); - private loadedFilePaths = new Set(); + private loadedFilePaths = new Map(); // Note: this will only contain StaticSymbols without members! private importAs = new Map(); @@ -63,19 +63,21 @@ export class AotSummaryResolver implements SummaryResolver { return this.host.fromSummaryFileName(fileName, referringLibFileName); } - resolveSummary(staticSymbol: StaticSymbol): Summary { + resolveSummary(staticSymbol: StaticSymbol): Summary|null { staticSymbol.assertNoMembers(); let summary = this.summaryCache.get(staticSymbol); if (!summary) { this._loadSummaryFile(staticSymbol.filePath); summary = this.summaryCache.get(staticSymbol) !; } - return summary; + return summary || null; } - getSymbolsOf(filePath: string): StaticSymbol[] { - this._loadSummaryFile(filePath); - return Array.from(this.summaryCache.keys()).filter((symbol) => symbol.filePath === filePath); + getSymbolsOf(filePath: string): StaticSymbol[]|null { + if (this._loadSummaryFile(filePath)) { + return Array.from(this.summaryCache.keys()).filter((symbol) => symbol.filePath === filePath); + } + return null; } getImportAs(staticSymbol: StaticSymbol): StaticSymbol { @@ -85,30 +87,33 @@ export class AotSummaryResolver implements SummaryResolver { addSummary(summary: Summary) { this.summaryCache.set(summary.symbol, summary); } - private _loadSummaryFile(filePath: string) { - if (this.loadedFilePaths.has(filePath)) { - return; + private _loadSummaryFile(filePath: string): boolean { + let hasSummary = this.loadedFilePaths.get(filePath); + if (hasSummary != null) { + return hasSummary; } - this.loadedFilePaths.add(filePath); + let json: string|null = null; if (this.isLibraryFile(filePath)) { const summaryFilePath = summaryFileName(filePath); - let json: string|null; try { json = this.host.loadSummary(summaryFilePath); } catch (e) { console.error(`Error loading summary file ${summaryFilePath}`); 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; } } diff --git a/packages/compiler/src/summary_resolver.ts b/packages/compiler/src/summary_resolver.ts index 1c7cd7a166..b247d2569f 100644 --- a/packages/compiler/src/summary_resolver.ts +++ b/packages/compiler/src/summary_resolver.ts @@ -19,7 +19,7 @@ export abstract class SummaryResolver { abstract toSummaryFileName(fileName: string, referringSrcFileName: string): string; abstract fromSummaryFileName(fileName: string, referringLibFileName: string): string; abstract resolveSummary(reference: T): Summary|null; - abstract getSymbolsOf(filePath: string): T[]; + abstract getSymbolsOf(filePath: string): T[]|null; abstract getImportAs(reference: T): T; abstract addSummary(summary: Summary): void; } diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts index 0f19f926d5..facf5f7f51 100644 --- a/packages/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/packages/compiler/test/aot/static_symbol_resolver_spec.ts @@ -136,15 +136,14 @@ describe('StaticSymbolResolver', () => { ]); }); - it('should merge the exported symbols of a file with the exported symbols of its summary', () => { - const someSymbol = symbolCache.get('/test.ts', 'a'); - init( - {'/test.ts': 'export var b = 2'}, - [{symbol: symbolCache.get('/test.ts', 'a'), metadata: 1}]); - expect(symbolResolver.getSymbolsOf('/test.ts')).toEqual([ - symbolCache.get('/test.ts', 'a'), symbolCache.get('/test.ts', 'b') - ]); - }); + 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( + {'/test.ts': 'export var b = 2'}, + [{symbol: symbolCache.get('/test.ts', 'a'), metadata: 1}]); + expect(symbolResolver.getSymbolsOf('/test.ts')).toEqual([symbolCache.get('/test.ts', 'a')]); + }); describe('importAs', () => { @@ -385,9 +384,10 @@ export class MockSummaryResolver implements SummaryResolver { resolveSummary(reference: StaticSymbol): Summary { return this.summaries.find(summary => summary.symbol === reference); }; - getSymbolsOf(filePath: string): StaticSymbol[] { - return this.summaries.filter(summary => summary.symbol.filePath === filePath) - .map(summary => summary.symbol); + getSymbolsOf(filePath: string): StaticSymbol[]|null { + const symbols = this.summaries.filter(summary => summary.symbol.filePath === filePath) + .map(summary => summary.symbol); + return symbols.length ? symbols : null; } getImportAs(symbol: StaticSymbol): StaticSymbol { const entry = this.importAs.find(entry => entry.symbol === symbol);