diff --git a/packages/compiler/src/aot/summary_serializer.ts b/packages/compiler/src/aot/summary_serializer.ts index 165c803cbf..8d6cfac961 100644 --- a/packages/compiler/src/aot/summary_serializer.ts +++ b/packages/compiler/src/aot/summary_serializer.ts @@ -28,54 +28,21 @@ export function serializeSummaries( // (we keep the statics though), as the class metadata is contained in the // CompileTypeSummary. symbols.forEach( - (resolvedSymbol) => toJsonSerializer.addOrMergeSummary( + (resolvedSymbol) => toJsonSerializer.addSummary( {symbol: resolvedSymbol.symbol, metadata: resolvedSymbol.metadata})); - // Add summaries that are referenced by the given symbols (transitively) - // Note: the serializer.symbols array might be growing while - // we execute the loop! - for (let processedIndex = 0; processedIndex < toJsonSerializer.symbols.length; processedIndex++) { - const symbol = toJsonSerializer.symbols[processedIndex]; - if (summaryResolver.isLibraryFile(symbol.filePath)) { - let summary = summaryResolver.resolveSummary(symbol); - if (!summary) { - // some symbols might originate from a plain typescript library - // that just exported .d.ts and .metadata.json files, i.e. where no summary - // files were created. - const resolvedSymbol = symbolResolver.resolveSymbol(symbol); - if (resolvedSymbol) { - summary = {symbol: resolvedSymbol.symbol, metadata: resolvedSymbol.metadata}; - } - } - if (summary) { - if (summary.type) { - forJitSerializer.addLibType(summary.type); - } - toJsonSerializer.addOrMergeSummary(summary); - } - } - } // Add type summaries. - // Note: We don't add the summaries of all referenced symbols as for the ResolvedSymbols, - // as the type summaries already contain the transitive data that they require - // (in a minimal way). types.forEach(({summary, metadata}) => { forJitSerializer.addSourceType(summary, metadata); - toJsonSerializer.addOrMergeSummary( - {symbol: summary.type.reference, metadata: null, type: summary}); - if (summary.summaryKind === CompileSummaryKind.NgModule) { - const ngModuleSummary = summary; - ngModuleSummary.exportedDirectives.concat(ngModuleSummary.exportedPipes).forEach((id) => { - const symbol: StaticSymbol = id.reference; - if (summaryResolver.isLibraryFile(symbol.filePath)) { - const summary = summaryResolver.resolveSummary(symbol); - if (summary) { - toJsonSerializer.addOrMergeSummary(summary); - } - } - }); + toJsonSerializer.addSummary( + {symbol: summary.type.reference, metadata: undefined, type: summary}); + }); + toJsonSerializer.unprocessedSymbolSummariesBySymbol.forEach((summary) => { + if (summaryResolver.isLibraryFile(summary.symbol.filePath) && summary.type) { + forJitSerializer.addLibType(summary.type); } }); + const {json, exportAs} = toJsonSerializer.serialize(srcFileName); forJitSerializer.serialize(exportAs); return {json, exportAs}; @@ -102,54 +69,81 @@ function createSummaryForJitFunction( ])); } +const enum SerializationFlags { + None = 0, + ResolveValue = 1, +} + class ToJsonSerializer extends ValueTransformer { // Note: This only contains symbols without members. - symbols: StaticSymbol[] = []; + private symbols: StaticSymbol[] = []; private indexBySymbol = new Map(); // This now contains a `__symbol: number` in the place of // StaticSymbols, but otherwise has the same shape as the original objects. private processedSummaryBySymbol = new Map(); private processedSummaries: any[] = []; + unprocessedSymbolSummariesBySymbol = new Map>(); + constructor( private symbolResolver: StaticSymbolResolver, private summaryResolver: SummaryResolver) { super(); } - addOrMergeSummary(summary: Summary) { - let symbolMeta = summary.metadata; - if (symbolMeta && symbolMeta.__symbolic === 'class') { - // For classes, we keep everything except their class decorators. - // We need to keep e.g. the ctor args, method names, method decorators - // so that the class can be extended in another compilation unit. - // We don't keep the class decorators as - // 1) they refer to data - // that should not cause a rebuild of downstream compilation units - // (e.g. inline templates of @Component, or @NgModule.declarations) - // 2) their data is already captured in TypeSummaries, e.g. DirectiveSummary. - const clone: {[key: string]: any} = {}; - Object.keys(symbolMeta).forEach((propName) => { - if (propName !== 'decorators') { - clone[propName] = symbolMeta[propName]; - } - }); - symbolMeta = clone; - } - + addSummary(summary: Summary) { + let unprocessedSummary = this.unprocessedSymbolSummariesBySymbol.get(summary.symbol); let processedSummary = this.processedSummaryBySymbol.get(summary.symbol); - if (!processedSummary) { - processedSummary = this.processValue({symbol: summary.symbol}); + if (!unprocessedSummary) { + unprocessedSummary = {symbol: summary.symbol, metadata: undefined}; + this.unprocessedSymbolSummariesBySymbol.set(summary.symbol, unprocessedSummary); + processedSummary = {symbol: this.processValue(summary.symbol, SerializationFlags.None)}; this.processedSummaries.push(processedSummary); this.processedSummaryBySymbol.set(summary.symbol, processedSummary); } - // Note: == on purpose to compare with undefined! - if (processedSummary.metadata == null && symbolMeta != null) { - processedSummary.metadata = this.processValue(symbolMeta); + if (!unprocessedSummary.metadata && summary.metadata) { + let metadata = summary.metadata || {}; + if (metadata.__symbolic === 'class') { + // For classes, we keep everything except their class decorators. + // We need to keep e.g. the ctor args, method names, method decorators + // so that the class can be extended in another compilation unit. + // We don't keep the class decorators as + // 1) they refer to data + // that should not cause a rebuild of downstream compilation units + // (e.g. inline templates of @Component, or @NgModule.declarations) + // 2) their data is already captured in TypeSummaries, e.g. DirectiveSummary. + const clone: {[key: string]: any} = {}; + Object.keys(metadata).forEach((propName) => { + if (propName !== 'decorators') { + clone[propName] = metadata[propName]; + } + }); + metadata = clone; + } + unprocessedSummary.metadata = metadata; + processedSummary.metadata = this.processValue(metadata, SerializationFlags.ResolveValue); } - // Note: == on purpose to compare with undefined! - if (processedSummary.type == null && summary.type != null) { - processedSummary.type = this.processValue(summary.type); + if (!unprocessedSummary.type && summary.type) { + unprocessedSummary.type = summary.type; + // Note: We don't add the summaries of all referenced symbols as for the ResolvedSymbols, + // as the type summaries already contain the transitive data that they require + // (in a minimal way). + processedSummary.type = this.processValue(summary.type, SerializationFlags.None); + // except for reexported directives / pipes, so we need to store + // their summaries explicitly. + if (summary.type.summaryKind === CompileSummaryKind.NgModule) { + const ngModuleSummary = summary.type; + ngModuleSummary.exportedDirectives.concat(ngModuleSummary.exportedPipes).forEach((id) => { + const symbol: StaticSymbol = id.reference; + if (this.summaryResolver.isLibraryFile(symbol.filePath) && + !this.unprocessedSymbolSummariesBySymbol.has(symbol)) { + const summary = this.summaryResolver.resolveSummary(symbol); + if (summary) { + this.addSummary(summary); + } + } + }); + } } } @@ -162,8 +156,11 @@ class ToJsonSerializer extends ValueTransformer { symbol.assertNoMembers(); let importAs: string = undefined !; if (this.summaryResolver.isLibraryFile(symbol.filePath)) { - importAs = `${symbol.name}_${index}`; - exportAs.push({symbol, exportAs: importAs}); + const summary = this.unprocessedSymbolSummariesBySymbol.get(symbol); + if (!summary || !summary.metadata || summary.metadata.__symbolic !== 'interface') { + importAs = `${symbol.name}_${index}`; + exportAs.push({symbol, exportAs: importAs}); + } } return { __symbol: index, @@ -176,21 +173,69 @@ class ToJsonSerializer extends ValueTransformer { return {json, exportAs}; } - private processValue(value: any): any { return visitValue(value, this, null); } + private processValue(value: any, flags: SerializationFlags): any { + return visitValue(value, this, flags); + } visitOther(value: any, context: any): any { if (value instanceof StaticSymbol) { - const baseSymbol = this.symbolResolver.getStaticSymbol(value.filePath, value.name); - let index = this.indexBySymbol.get(baseSymbol); - // Note: == on purpose to compare with undefined! - if (index == null) { - index = this.indexBySymbol.size; - this.indexBySymbol.set(baseSymbol, index); - this.symbols.push(baseSymbol); - } + let baseSymbol = this.symbolResolver.getStaticSymbol(value.filePath, value.name); + const index = this.visitStaticSymbol(baseSymbol, context); return {__symbol: index, members: value.members}; } } + + /** + * Returns null if the options.resolveValue is true, and the summary for the symbol + * resolved to a type or could not be resolved. + */ + private visitStaticSymbol(baseSymbol: StaticSymbol, flags: SerializationFlags): number { + let index: number|undefined|null = this.indexBySymbol.get(baseSymbol); + let summary: Summary|null = null; + if (flags & SerializationFlags.ResolveValue && + this.summaryResolver.isLibraryFile(baseSymbol.filePath)) { + if (this.unprocessedSymbolSummariesBySymbol.has(baseSymbol)) { + // the summary for this symbol was already added + // -> nothing to do. + return index !; + } + summary = this.loadSummary(baseSymbol); + if (summary && summary.metadata instanceof StaticSymbol) { + // The summary is a reexport + index = this.visitStaticSymbol(summary.metadata, flags); + // reset the summary as it is just a reexport, so we don't want to store it. + summary = null; + } + } else if (index != null) { + // Note: == on purpose to compare with undefined! + // No summary and the symbol is already added -> nothing to do. + return index; + } + // Note: == on purpose to compare with undefined! + if (index == null) { + index = this.symbols.length; + this.symbols.push(baseSymbol); + } + this.indexBySymbol.set(baseSymbol, index); + if (summary) { + this.addSummary(summary); + } + return index; + } + + private loadSummary(symbol: StaticSymbol): Summary|null { + let summary = this.summaryResolver.resolveSummary(symbol); + if (!summary) { + // some symbols might originate from a plain typescript library + // that just exported .d.ts and .metadata.json files, i.e. where no summary + // files were created. + const resolvedSymbol = this.symbolResolver.resolveSymbol(symbol); + if (resolvedSymbol) { + summary = {symbol: resolvedSymbol.symbol, metadata: resolvedSymbol.metadata}; + } + } + return summary; + } } class ForJitSerializer { diff --git a/packages/compiler/test/aot/compiler_spec.ts b/packages/compiler/test/aot/compiler_spec.ts index fe081e3f0a..ef08c70c94 100644 --- a/packages/compiler/test/aot/compiler_spec.ts +++ b/packages/compiler/test/aot/compiler_spec.ts @@ -6,14 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {GeneratedFile, toTypeScript} from '@angular/compiler'; +import {AotSummaryResolver, GeneratedFile, StaticSymbolCache, StaticSymbolResolver, toTypeScript} from '@angular/compiler'; import {NodeFlags} from '@angular/core/src/view/index'; import {MetadataBundler, privateEntriesToIndex} from '@angular/tsc-wrapped'; import * as ts from 'typescript'; import {extractSourceMap, originalPositionFor} from '../output/source_map_util'; -import {EmittingCompilerHost, MockDirectory, MockMetadataBundlerHost, arrayToMockDir, compile, expectNoDiagnostics, settings, setup, toMockFileArray} from './test_util'; +import {EmittingCompilerHost, MockAotCompilerHost, MockCompilerHost, MockDirectory, MockMetadataBundlerHost, arrayToMockDir, compile, expectNoDiagnostics, settings, setup, toMockFileArray} from './test_util'; describe('compiler (unbundled Angular)', () => { let angularFiles = setup(); @@ -458,12 +458,43 @@ describe('compiler (unbundled Angular)', () => { }); }); - describe('inheritance with summaries', () => { + describe('summaries', () => { let angularSummaryFiles: MockDirectory; - beforeEach(() => { + beforeAll(() => { angularSummaryFiles = compile(angularFiles, {useSummaries: false, emit: true}).outDir; }); + inheritanceWithSummariesSpecs(() => angularSummaryFiles); + + it('should not reexport type symbols mentioned in constructors', () => { + const libInput: MockDirectory = { + 'lib': { + 'base.ts': ` + export type AType = {}; + + export class AClass { + constructor(a: AType) {} + } + ` + } + }; + const appInput: MockDirectory = { + 'app': { + 'main.ts': ` + export * from '../lib/base'; + ` + } + }; + + const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true}); + const {genFiles: appGenFiles} = + compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true}); + const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts'); + expect(toTypeScript(appNgFactory)).not.toContain('AType'); + }); + }); + + function inheritanceWithSummariesSpecs(getAngularSummaryFiles: () => MockDirectory) { function compileParentAndChild( {parentClassDecorator, parentModuleDecorator, childClassDecorator, childModuleDecorator}: { parentClassDecorator: string, @@ -499,8 +530,10 @@ describe('compiler (unbundled Angular)', () => { } }; - const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true}); - const {genFiles} = compile([libOutDir, appInput, angularSummaryFiles], {useSummaries: true}); + const {outDir: libOutDir} = + compile([libInput, getAngularSummaryFiles()], {useSummaries: true}); + const {genFiles} = + compile([libOutDir, appInput, getAngularSummaryFiles()], {useSummaries: true}); return genFiles.find(gf => gf.srcFileUrl === '/app/main.ts'); } @@ -534,8 +567,10 @@ describe('compiler (unbundled Angular)', () => { } }; - const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true}); - const {genFiles} = compile([libOutDir, appInput, angularSummaryFiles], {useSummaries: true}); + const {outDir: libOutDir} = + compile([libInput, getAngularSummaryFiles()], {useSummaries: true}); + const {genFiles} = + compile([libOutDir, appInput, getAngularSummaryFiles()], {useSummaries: true}); const mainNgFactory = genFiles.find(gf => gf.srcFileUrl === '/app/main.ts'); const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy; expect(toTypeScript(mainNgFactory)) @@ -584,11 +619,11 @@ describe('compiler (unbundled Angular)', () => { } }; const {outDir: lib1OutDir} = - compile([lib1Input, angularSummaryFiles], {useSummaries: true}); + compile([lib1Input, getAngularSummaryFiles()], {useSummaries: true}); const {outDir: lib2OutDir} = - compile([lib1OutDir, lib2Input, angularSummaryFiles], {useSummaries: true}); + compile([lib1OutDir, lib2Input, getAngularSummaryFiles()], {useSummaries: true}); const {genFiles} = - compile([lib2OutDir, appInput, angularSummaryFiles], {useSummaries: true}); + compile([lib2OutDir, appInput, getAngularSummaryFiles()], {useSummaries: true}); const mainNgFactory = genFiles.find(gf => gf.srcFileUrl === '/app/main.ts'); const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy; expect(toTypeScript(mainNgFactory)) @@ -714,7 +749,7 @@ describe('compiler (unbundled Angular)', () => { 'Please add a NgModule decorator to the class.'); }); }); - }); + } }); describe('compiler (bundled Angular)', () => { diff --git a/packages/compiler/test/aot/summary_serializer_spec.ts b/packages/compiler/test/aot/summary_serializer_spec.ts index 90e8c36d06..dfcf38cada 100644 --- a/packages/compiler/test/aot/summary_serializer_spec.ts +++ b/packages/compiler/test/aot/summary_serializer_spec.ts @@ -108,7 +108,12 @@ export function main() { () => { init(); const externalSerialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [], [ + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + {symbol: symbolCache.get('/tmp/external.ts', 'SomeExternalPipe'), metadata: null}, + {symbol: symbolCache.get('/tmp/external.ts', 'SomeExternalDir'), metadata: null}, + ], + [ { summary: { summaryKind: CompileSummaryKind.Pipe, @@ -135,7 +140,11 @@ export function main() { }); const serialized = serializeSummaries( - 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [], [{ + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + {symbol: symbolCache.get('/tmp/some_module.ts', 'SomeModule'), metadata: null}, + ], + [{ summary: { summaryKind: CompileSummaryKind.NgModule, type: {reference: symbolCache.get('/tmp/some_module.ts', 'SomeModule')}, @@ -152,15 +161,42 @@ export function main() { }, metadata: null as any }]); - const summaries = deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) .summaries; + init({ + '/tmp/some_module.ngsummary.json': serialized.json, + }); + + const serializedReexport = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + { + symbol: symbolCache.get('/tmp/some_reexport.ts', 'ReexportModule'), + metadata: symbolCache.get('/tmp/some_module.d.ts', 'SomeModule') + }, + ], + []); + expect(summaries.length).toBe(3); expect(summaries[0].symbol).toBe(symbolCache.get('/tmp/some_module.d.ts', 'SomeModule')); expect(summaries[1].symbol).toBe(symbolCache.get('/tmp/external.d.ts', 'SomeExternalDir')); expect(summaries[2].symbol) .toBe(symbolCache.get('/tmp/external.d.ts', 'SomeExternalPipe')); + + const reexportSummaries = + deserializeSummaries( + symbolCache, summaryResolver, 'someFile.d.ts', serializedReexport.json) + .summaries; + expect(reexportSummaries.length).toBe(4); + expect(reexportSummaries[0].symbol) + .toBe(symbolCache.get('/tmp/some_reexport.d.ts', 'ReexportModule')); + expect(reexportSummaries[1].symbol) + .toBe(symbolCache.get('/tmp/some_module.d.ts', 'SomeModule')); + expect(reexportSummaries[2].symbol) + .toBe(symbolCache.get('/tmp/external.d.ts', 'SomeExternalDir')); + expect(reexportSummaries[3].symbol) + .toBe(symbolCache.get('/tmp/external.d.ts', 'SomeExternalPipe')); }); it('should automatically add the metadata of referenced symbols that are not in the source files', @@ -176,6 +212,16 @@ export function main() { { symbol: symbolCache.get('/tmp/external_svc.ts', 'SomeService'), metadata: {__symbolic: 'class'} + }, + // Note: This is an important usecase when using ng1 and ng2 together via + // goog.module. + // In these cases, users write the following to get a referrable symbol in metadata + // collection: + // import UsernameService from 'goog:somePackage.UsernameService'; + // export {UsernameService}; + { + symbol: symbolCache.get('/tmp/external.ts', 'ReexportNonExistent'), + metadata: symbolCache.get('/tmp/external.ts', 'NonExistent'), } ], [{ @@ -204,7 +250,8 @@ export function main() { metadata: { local: symbolCache.get('/tmp/local.ts', 'local'), external: symbolCache.get('/tmp/external.d.ts', 'PROVIDERS'), - externalNonSummary: symbolCache.get('/tmp/non_summary.d.ts', 'external') + externalNonSummary: symbolCache.get('/tmp/non_summary.d.ts', 'external'), + reexportNonExistent: symbolCache.get('/tmp/external.ts', 'ReexportNonExistent'), } }], []); @@ -218,21 +265,87 @@ export function main() { expect(summaries[0].metadata).toEqual({ local: symbolCache.get('/tmp/local.d.ts', 'local'), external: symbolCache.get('/tmp/external.d.ts', 'PROVIDERS'), - externalNonSummary: symbolCache.get('/tmp/non_summary.d.ts', 'external') + externalNonSummary: symbolCache.get('/tmp/non_summary.d.ts', 'external'), + reexportNonExistent: symbolCache.get('/tmp/external.d.ts', 'ReexportNonExistent'), }); expect(summaries[1].symbol).toBe(symbolCache.get('/tmp/external.d.ts', 'PROVIDERS')); expect(summaries[1].metadata).toEqual([symbolCache.get( '/tmp/external_svc.d.ts', 'SomeService')]); + // SomService is a transitive dep, but should have been serialized as well. + expect(summaries[2].symbol).toBe(symbolCache.get('/tmp/external_svc.d.ts', 'SomeService')); + expect(summaries[2].type !.type.reference) + .toBe(symbolCache.get('/tmp/external_svc.d.ts', 'SomeService')); // there was no summary for non_summary, but it should have // been serialized as well. - expect(summaries[2].symbol).toBe(symbolCache.get('/tmp/non_summary.d.ts', 'external')); - expect(summaries[2].metadata).toEqual('b'); - // SomService is a transitive dep, but should have been serialized as well. - expect(summaries[3].symbol).toBe(symbolCache.get('/tmp/external_svc.d.ts', 'SomeService')); - expect(summaries[3].type !.type.reference) - .toBe(symbolCache.get('/tmp/external_svc.d.ts', 'SomeService')); + expect(summaries[3].symbol).toBe(symbolCache.get('/tmp/non_summary.d.ts', 'external')); + expect(summaries[3].metadata).toEqual('b'); }); + it('should resolve reexported values in libraries', () => { + init(); + const externalSerialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + {symbol: symbolCache.get('/tmp/external.ts', 'value'), metadata: 'someString'}, + { + symbol: symbolCache.get('/tmp/external.ts', 'reexportValue'), + metadata: symbolCache.get('/tmp/external.ts', 'value') + }, + ], + []); + init({ + '/tmp/external.ngsummary.json': externalSerialized.json, + }); + const serialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + { + symbol: symbolCache.get('/tmp/test.ts', 'mainValue'), + metadata: symbolCache.get('/tmp/external.d.ts', 'reexportValue'), + }, + ], + []); + + const summaries = + deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) + .summaries; + expect(summaries.length).toBe(2); + expect(summaries[0].symbol).toBe(symbolCache.get('/tmp/test.d.ts', 'mainValue')); + expect(summaries[0].metadata).toBe(symbolCache.get('/tmp/external.d.ts', 'value')); + expect(summaries[1].symbol).toBe(symbolCache.get('/tmp/external.d.ts', 'value')); + expect(summaries[1].metadata).toBe('someString'); + }); + + it('should not create "importAs" names for reexported types in libraries', () => { + init(); + const externalSerialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, + [ + { + symbol: symbolCache.get('/tmp/external.ts', 'type'), + metadata: {__symbolic: 'interface'} + }, + { + symbol: symbolCache.get('/tmp/external.ts', 'reexportType'), + metadata: symbolCache.get('/tmp/external.ts', 'type') + }, + ], + []); + init({ + '/tmp/external.ngsummary.json': externalSerialized.json, + }); + const serialized = serializeSummaries( + 'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{ + symbol: symbolCache.get('/tmp/test.ts', 'mainType'), + metadata: symbolCache.get('/tmp/external.d.ts', 'reexportType'), + }], + []); + const importAs = + deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json) + .importAs; + expect(importAs).toEqual([]); + }); + it('should create "importAs" names for non source symbols', () => { init(); const serialized = serializeSummaries(