From 8ef621ad2ad5662de56af1b5eb8ed2bff45809c2 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Thu, 30 Mar 2017 14:51:29 -0700 Subject: [PATCH] fix(compiler): fix inheritance for AOT with summaries (#15583) Allows to inherit ctor args, lifecycle hooks and statics from a class in another compilation unit. Will error if trying to inherit from a class in another compilation unit that has an `@Component` / `@Directive` / `@Pipe` / `@NgModule`. --- packages/compiler-cli/src/ngtools_api.ts | 2 +- packages/compiler/src/aot/compiler_factory.ts | 2 +- packages/compiler/src/aot/static_reflector.ts | 91 ++++-- .../src/aot/static_symbol_resolver.ts | 11 + .../compiler/src/aot/summary_serializer.ts | 21 +- packages/compiler/src/i18n/extractor.ts | 2 +- packages/compiler/src/metadata_resolver.ts | 12 +- packages/compiler/test/aot/compiler_spec.ts | 295 +++++++++++++++++- .../test/aot/static_reflector_spec.ts | 7 +- .../test/aot/static_symbol_resolver_spec.ts | 25 ++ .../test/aot/summary_serializer_spec.ts | 11 +- packages/compiler/test/aot/test_util.ts | 4 +- .../language-service/src/typescript_host.ts | 8 +- 13 files changed, 436 insertions(+), 55 deletions(-) diff --git a/packages/compiler-cli/src/ngtools_api.ts b/packages/compiler-cli/src/ngtools_api.ts index 2e180e2f33..d602f6843a 100644 --- a/packages/compiler-cli/src/ngtools_api.ts +++ b/packages/compiler-cli/src/ngtools_api.ts @@ -124,7 +124,7 @@ export class NgTools_InternalApi_NG_2 { const symbolCache = new StaticSymbolCache(); const summaryResolver = new AotSummaryResolver(ngCompilerHost, symbolCache); const symbolResolver = new StaticSymbolResolver(ngCompilerHost, symbolCache, summaryResolver); - const staticReflector = new StaticReflector(symbolResolver); + const staticReflector = new StaticReflector(summaryResolver, symbolResolver); const routeMap = listLazyRoutesOfModule(options.entryModule, ngCompilerHost, staticReflector); return Object.keys(routeMap).reduce( diff --git a/packages/compiler/src/aot/compiler_factory.ts b/packages/compiler/src/aot/compiler_factory.ts index 04c66ba638..56b7990416 100644 --- a/packages/compiler/src/aot/compiler_factory.ts +++ b/packages/compiler/src/aot/compiler_factory.ts @@ -47,7 +47,7 @@ export function createAotCompiler(compilerHost: AotCompilerHost, options: AotCom const symbolCache = new StaticSymbolCache(); const summaryResolver = new AotSummaryResolver(compilerHost, symbolCache); const symbolResolver = new StaticSymbolResolver(compilerHost, symbolCache, summaryResolver); - const staticReflector = new StaticReflector(symbolResolver); + const staticReflector = new StaticReflector(summaryResolver, symbolResolver); StaticAndDynamicReflectionCapabilities.install(staticReflector); const console = new Console(); const htmlParser = new I18NHtmlParser( diff --git a/packages/compiler/src/aot/static_reflector.ts b/packages/compiler/src/aot/static_reflector.ts index edbda293f5..513252b8d5 100644 --- a/packages/compiler/src/aot/static_reflector.ts +++ b/packages/compiler/src/aot/static_reflector.ts @@ -7,7 +7,11 @@ */ import {Attribute, Component, ContentChild, ContentChildren, Directive, Host, HostBinding, HostListener, Inject, Injectable, Input, NgModule, Optional, Output, Pipe, Self, SkipSelf, ViewChild, ViewChildren, animate, group, keyframes, sequence, state, style, transition, trigger, ɵReflectorReader} from '@angular/core'; + +import {CompileSummaryKind} from '../compile_metadata'; +import {SummaryResolver} from '../summary_resolver'; import {syntaxError} from '../util'; + import {StaticSymbol} from './static_symbol'; import {StaticSymbolResolver} from './static_symbol_resolver'; @@ -35,8 +39,11 @@ export class StaticReflector implements ɵReflectorReader { private conversionMap = new Map any>(); private injectionToken: StaticSymbol; private opaqueToken: StaticSymbol; + private annotationForParentClassWithSummaryKind = new Map(); + private annotationNames = new Map(); constructor( + private summaryResolver: SummaryResolver, private symbolResolver: StaticSymbolResolver, knownMetadataClasses: {name: string, filePath: string, ctor: any}[] = [], knownMetadataFunctions: {name: string, filePath: string, fn: any}[] = [], @@ -47,6 +54,17 @@ export class StaticReflector implements ɵReflectorReader { this.getStaticSymbol(kc.filePath, kc.name), kc.ctor)); knownMetadataFunctions.forEach( (kf) => this._registerFunction(this.getStaticSymbol(kf.filePath, kf.name), kf.fn)); + this.annotationForParentClassWithSummaryKind.set( + CompileSummaryKind.Directive, [Directive, Component]); + this.annotationForParentClassWithSummaryKind.set(CompileSummaryKind.Pipe, [Pipe]); + this.annotationForParentClassWithSummaryKind.set(CompileSummaryKind.NgModule, [NgModule]); + this.annotationForParentClassWithSummaryKind.set( + CompileSummaryKind.Injectable, [Injectable, Pipe, Directive, Component, NgModule]); + this.annotationNames.set(Directive, 'Directive'); + this.annotationNames.set(Component, 'Component'); + this.annotationNames.set(Pipe, 'Pipe'); + this.annotationNames.set(NgModule, 'NgModule'); + this.annotationNames.set(Injectable, 'Injectable'); } importUri(typeOrFunc: StaticSymbol): string { @@ -96,17 +114,33 @@ export class StaticReflector implements ɵReflectorReader { if (!annotations) { annotations = []; const classMetadata = this.getTypeMetadata(type); - if (classMetadata['extends']) { - const parentType = this.trySimplify(type, classMetadata['extends']); - if (parentType && (parentType instanceof StaticSymbol)) { - const parentAnnotations = this.annotations(parentType); - annotations.push(...parentAnnotations); - } + const parentType = this.findParentType(type, classMetadata); + if (parentType) { + const parentAnnotations = this.annotations(parentType); + annotations.push(...parentAnnotations); } + let ownAnnotations: any[] = []; if (classMetadata['decorators']) { - const ownAnnotations: any[] = this.simplify(type, classMetadata['decorators']); + ownAnnotations = this.simplify(type, classMetadata['decorators']); annotations.push(...ownAnnotations); } + if (parentType && !this.summaryResolver.isLibraryFile(type.filePath) && + this.summaryResolver.isLibraryFile(parentType.filePath)) { + const summary = this.summaryResolver.resolveSummary(parentType); + if (summary && summary.type) { + const requiredAnnotationTypes = + this.annotationForParentClassWithSummaryKind.get(summary.type.summaryKind); + const typeHasRequiredAnnotation = requiredAnnotationTypes.some( + requiredType => ownAnnotations.some(ann => ann instanceof requiredType)); + if (!typeHasRequiredAnnotation) { + this.reportError( + syntaxError( + `Class ${type.name} in ${type.filePath} extends from a ${CompileSummaryKind[summary.type.summaryKind]} in another compilation unit without duplicating the decorator. ` + + `Please add a ${requiredAnnotationTypes.map(type => this.annotationNames.get(type)).join(' or ')} decorator to the class.`), + type); + } + } + } this.annotationCache.set(type, annotations.filter(ann => !!ann)); } return annotations; @@ -117,14 +151,12 @@ export class StaticReflector implements ɵReflectorReader { if (!propMetadata) { const classMetadata = this.getTypeMetadata(type); propMetadata = {}; - if (classMetadata['extends']) { - const parentType = this.trySimplify(type, classMetadata['extends']); - if (parentType instanceof StaticSymbol) { - const parentPropMetadata = this.propMetadata(parentType); - Object.keys(parentPropMetadata).forEach((parentProp) => { - propMetadata[parentProp] = parentPropMetadata[parentProp]; - }); - } + const parentType = this.findParentType(type, classMetadata); + if (parentType) { + const parentPropMetadata = this.propMetadata(parentType); + Object.keys(parentPropMetadata).forEach((parentProp) => { + propMetadata[parentProp] = parentPropMetadata[parentProp]; + }); } const members = classMetadata['members'] || {}; @@ -157,6 +189,7 @@ export class StaticReflector implements ɵReflectorReader { let parameters = this.parameterCache.get(type); if (!parameters) { const classMetadata = this.getTypeMetadata(type); + const parentType = this.findParentType(type, classMetadata); const members = classMetadata ? classMetadata['members'] : null; const ctorData = members ? members['__ctor__'] : null; if (ctorData) { @@ -175,11 +208,8 @@ export class StaticReflector implements ɵReflectorReader { } parameters.push(nestedResult); }); - } else if (classMetadata['extends']) { - const parentType = this.trySimplify(type, classMetadata['extends']); - if (parentType instanceof StaticSymbol) { - parameters = this.parameters(parentType); - } + } else if (parentType) { + parameters = this.parameters(parentType); } if (!parameters) { parameters = []; @@ -198,14 +228,12 @@ export class StaticReflector implements ɵReflectorReader { if (!methodNames) { const classMetadata = this.getTypeMetadata(type); methodNames = {}; - if (classMetadata['extends']) { - const parentType = this.trySimplify(type, classMetadata['extends']); - if (parentType instanceof StaticSymbol) { - const parentMethodNames = this._methodNames(parentType); - Object.keys(parentMethodNames).forEach((parentProp) => { - methodNames[parentProp] = parentMethodNames[parentProp]; - }); - } + const parentType = this.findParentType(type, classMetadata); + if (parentType) { + const parentMethodNames = this._methodNames(parentType); + Object.keys(parentMethodNames).forEach((parentProp) => { + methodNames[parentProp] = parentMethodNames[parentProp]; + }); } const members = classMetadata['members'] || {}; @@ -219,6 +247,13 @@ export class StaticReflector implements ɵReflectorReader { return methodNames; } + private findParentType(type: StaticSymbol, classMetadata: any): StaticSymbol|null { + const parentType = this.trySimplify(type, classMetadata['extends']); + if (parentType instanceof StaticSymbol) { + return parentType; + } + } + hasLifecycleHook(type: any, lcProperty: string): boolean { if (!(type instanceof StaticSymbol)) { this.reportError( diff --git a/packages/compiler/src/aot/static_symbol_resolver.ts b/packages/compiler/src/aot/static_symbol_resolver.ts index d451a961eb..7135f6f744 100644 --- a/packages/compiler/src/aot/static_symbol_resolver.ts +++ b/packages/compiler/src/aot/static_symbol_resolver.ts @@ -307,6 +307,17 @@ export class StaticSymbolResolver { private createResolvedSymbol( sourceSymbol: StaticSymbol, topLevelPath: string, topLevelSymbolNames: Set, metadata: any): ResolvedStaticSymbol { + // For classes that don't have Angular summaries / metadata, + // we only keep their arity, but nothing else + // (e.g. their constructor parameters). + // We do this to prevent introducing deep imports + // as we didn't generate .ngfactory.ts files with proper reexports. + if (this.summaryResolver.isLibraryFile(sourceSymbol.filePath) && metadata && + metadata['__symbolic'] === 'class') { + const transformedMeta = {__symbolic: 'class', arity: metadata.arity}; + return new ResolvedStaticSymbol(sourceSymbol, transformedMeta); + } + const self = this; class ReferenceTransformer extends ValueTransformer { diff --git a/packages/compiler/src/aot/summary_serializer.ts b/packages/compiler/src/aot/summary_serializer.ts index 6bf3011dd8..81bcae1458 100644 --- a/packages/compiler/src/aot/summary_serializer.ts +++ b/packages/compiler/src/aot/summary_serializer.ts @@ -53,7 +53,7 @@ export function serializeSummaries( // (in a minimal way). types.forEach((typeSummary) => { serializer.addOrMergeSummary( - {symbol: typeSummary.type.reference, metadata: {__symbolic: 'class'}, type: typeSummary}); + {symbol: typeSummary.type.reference, metadata: null, type: typeSummary}); if (typeSummary.summaryKind === CompileSummaryKind.NgModule) { const ngModuleSummary = typeSummary; ngModuleSummary.exportedDirectives.concat(ngModuleSummary.exportedPipes).forEach((id) => { @@ -94,10 +94,21 @@ class Serializer extends ValueTransformer { addOrMergeSummary(summary: Summary) { let symbolMeta = summary.metadata; if (symbolMeta && symbolMeta.__symbolic === 'class') { - // For classes, we only keep their statics and arity, but not the metadata - // of the class itself as that has been captured already via other summaries - // (e.g. DirectiveSummary, ...). - symbolMeta = {__symbolic: 'class', statics: symbolMeta.statics, arity: symbolMeta.arity}; + // 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; } let processedSummary = this.processedSummaryBySymbol.get(summary.symbol); diff --git a/packages/compiler/src/i18n/extractor.ts b/packages/compiler/src/i18n/extractor.ts index b8c83a21e8..104841ef43 100644 --- a/packages/compiler/src/i18n/extractor.ts +++ b/packages/compiler/src/i18n/extractor.ts @@ -94,7 +94,7 @@ export class Extractor { const symbolCache = new StaticSymbolCache(); const summaryResolver = new AotSummaryResolver(host, symbolCache); const staticSymbolResolver = new StaticSymbolResolver(host, symbolCache, summaryResolver); - const staticReflector = new StaticReflector(staticSymbolResolver); + const staticReflector = new StaticReflector(summaryResolver, staticSymbolResolver); StaticAndDynamicReflectionCapabilities.install(staticReflector); const config = diff --git a/packages/compiler/src/metadata_resolver.ts b/packages/compiler/src/metadata_resolver.ts index dd84cd0f6e..eee5ac8885 100644 --- a/packages/compiler/src/metadata_resolver.ts +++ b/packages/compiler/src/metadata_resolver.ts @@ -933,14 +933,12 @@ export class CompileMetadataResolver { const dirMeta = this.getNonNormalizedDirectiveMetadata(dirType); if (dirMeta && dirMeta.metadata.isComponent) { return {componentType: dirType, componentFactory: dirMeta.metadata.componentFactory}; - } else { - const dirSummary = - this._loadSummary(dirType, cpl.CompileSummaryKind.Directive); - if (dirSummary && dirSummary.isComponent) { - return {componentType: dirType, componentFactory: dirSummary.componentFactory}; - } } - + const dirSummary = + this._loadSummary(dirType, cpl.CompileSummaryKind.Directive); + if (dirSummary && dirSummary.isComponent) { + return {componentType: dirType, componentFactory: dirSummary.componentFactory}; + } if (throwIfNotFound) { throw syntaxError(`${dirType.name} cannot be used as an entry component.`); } diff --git a/packages/compiler/test/aot/compiler_spec.ts b/packages/compiler/test/aot/compiler_spec.ts index 51cd5aa39e..c35017754a 100644 --- a/packages/compiler/test/aot/compiler_spec.ts +++ b/packages/compiler/test/aot/compiler_spec.ts @@ -8,6 +8,7 @@ import {AotCompiler, AotCompilerHost, AotCompilerOptions, GeneratedFile, createAotCompiler} from '@angular/compiler'; import {RenderComponentType, ɵReflectionCapabilities as ReflectionCapabilities, ɵreflector as reflector} from '@angular/core'; +import {NodeFlags} from '@angular/core/src/view/index'; import {async} from '@angular/core/testing'; import {MetadataBundler, MetadataCollector, ModuleMetadata, privateEntriesToIndex} from '@angular/tsc-wrapped'; import * as ts from 'typescript'; @@ -368,6 +369,293 @@ describe('compiler (unbundled Angular)', () => { })); }); + + describe('inheritance with summaries', () => { + function compileWithSummaries( + libInput: MockData, appInput: MockData): Promise { + const libHost = new MockCompilerHost(['/lib/base.ts'], libInput, angularFiles); + const libAotHost = new MockAotCompilerHost(libHost); + libAotHost.tsFilesOnly(); + const appHost = new MockCompilerHost(['/app/main.ts'], appInput, angularFiles); + const appAotHost = new MockAotCompilerHost(appHost); + appAotHost.tsFilesOnly(); + return compile(libHost, libAotHost, expectNoDiagnostics, expectNoDiagnosticsAndEmit) + .then(() => { + libHost.writtenFiles.forEach((value, key) => appHost.writeFile(key, value, false)); + libHost.overrides.forEach((value, key) => appHost.override(key, value)); + + return compile(appHost, appAotHost, expectNoDiagnostics, expectNoDiagnosticsAndEmit); + }); + } + + function compileParentAndChild( + {parentClassDecorator, parentModuleDecorator, childClassDecorator, childModuleDecorator}: { + parentClassDecorator: string, + parentModuleDecorator: string, + childClassDecorator: string, + childModuleDecorator: string + }) { + const libInput: MockData = { + 'lib': { + 'base.ts': ` + import {Injectable, Pipe, Directive, Component, NgModule} from '@angular/core'; + + ${parentClassDecorator} + export class Base {} + + ${parentModuleDecorator} + export class BaseModule {} + ` + } + }; + const appInput: MockData = { + 'app': { + 'main.ts': ` + import {Injectable, Pipe, Directive, Component, NgModule} from '@angular/core'; + import {Base} from '../lib/base'; + + ${childClassDecorator} + export class Extends extends Base {} + + ${childModuleDecorator} + export class MyModule {} + ` + } + }; + + return compileWithSummaries(libInput, appInput) + .then((generatedFiles) => generatedFiles.find(gf => gf.srcFileUrl === '/app/main.ts')); + } + + it('should inherit ctor and lifecycle hooks from classes in other compilation units', + async(() => { + const libInput: MockData = { + 'lib': { + 'base.ts': ` + export class AParam {} + + export class Base { + constructor(a: AParam) {} + ngOnDestroy() {} + } + ` + } + }; + const appInput: MockData = { + 'app': { + 'main.ts': ` + import {NgModule, Component} from '@angular/core'; + import {Base} from '../lib/base'; + + @Component({template: ''}) + export class Extends extends Base {} + + @NgModule({ + declarations: [Extends] + }) + export class MyModule {} + ` + } + }; + + compileWithSummaries(libInput, appInput).then((generatedFiles) => { + const mainNgFactory = generatedFiles.find(gf => gf.srcFileUrl === '/app/main.ts'); + const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy; + expect(mainNgFactory.source) + .toContain(`${flags},(null as any),0,import1.Extends,[import2.AParam]`); + }); + })); + + it('should inherit ctor and lifecycle hooks from classes in other compilation units over 2 levels', + async(() => { + const lib1Input: MockData = { + 'lib1': { + 'base.ts': ` + export class AParam {} + + export class Base { + constructor(a: AParam) {} + ngOnDestroy() {} + } + ` + } + }; + + const lib2Input: MockData = { + 'lib2': { + 'middle.ts': ` + import {Base} from '../lib1/base'; + export class Middle extends Base {} + ` + } + }; + + + const appInput: MockData = { + 'app': { + 'main.ts': ` + import {NgModule, Component} from '@angular/core'; + import {Middle} from '../lib2/middle'; + + @Component({template: ''}) + export class Extends extends Middle {} + + @NgModule({ + declarations: [Extends] + }) + export class MyModule {} + ` + } + }; + const lib1Host = new MockCompilerHost(['/lib1/base.ts'], lib1Input, angularFiles); + const lib1AotHost = new MockAotCompilerHost(lib1Host); + lib1AotHost.tsFilesOnly(); + const lib2Host = new MockCompilerHost(['/lib2/middle.ts'], lib2Input, angularFiles); + const lib2AotHost = new MockAotCompilerHost(lib2Host); + lib2AotHost.tsFilesOnly(); + const appHost = new MockCompilerHost(['/app/main.ts'], appInput, angularFiles); + const appAotHost = new MockAotCompilerHost(appHost); + appAotHost.tsFilesOnly(); + compile(lib1Host, lib1AotHost, expectNoDiagnostics, expectNoDiagnosticsAndEmit) + .then(() => { + lib1Host.writtenFiles.forEach((value, key) => lib2Host.writeFile(key, value, false)); + lib1Host.overrides.forEach((value, key) => lib2Host.override(key, value)); + return compile( + lib2Host, lib2AotHost, expectNoDiagnostics, expectNoDiagnosticsAndEmit); + }) + .then(() => { + lib2Host.writtenFiles.forEach((value, key) => appHost.writeFile(key, value, false)); + lib2Host.overrides.forEach((value, key) => appHost.override(key, value)); + return compile(appHost, appAotHost, expectNoDiagnostics, expectNoDiagnosticsAndEmit); + }) + .then((generatedFiles) => { + const mainNgFactory = generatedFiles.find(gf => gf.srcFileUrl === '/app/main.ts'); + const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy; + expect(mainNgFactory.source) + .toContain(`${flags},(null as any),0,import1.Extends,[import2.AParam_2]`); + }); + })); + + describe('Injectable', () => { + it('should allow to inherit', async(() => { + compileParentAndChild({ + parentClassDecorator: '@Injectable()', + parentModuleDecorator: '@NgModule({providers: [Base]})', + childClassDecorator: '@Injectable()', + childModuleDecorator: '@NgModule({providers: [Extends]})', + }).then((mainNgFactory) => { expect(mainNgFactory).toBeTruthy(); }); + })); + + it('should error if the child class has no matching decorator', async(() => { + compileParentAndChild({ + parentClassDecorator: '@Injectable()', + parentModuleDecorator: '@NgModule({providers: [Base]})', + childClassDecorator: '', + childModuleDecorator: '@NgModule({providers: [Extends]})', + }).then(fail, (e) => { + expect(e.message).toContain( + 'Class Extends in /app/main.ts extends from a Injectable in another compilation unit without duplicating the decorator. ' + + 'Please add a Injectable or Pipe or Directive or Component or NgModule decorator to the class.'); + }); + })); + }); + + describe('Component', () => { + it('should allow to inherit', async(() => { + compileParentAndChild({ + parentClassDecorator: `@Component({template: ''})`, + parentModuleDecorator: '@NgModule({declarations: [Base]})', + childClassDecorator: `@Component({template: ''})`, + childModuleDecorator: '@NgModule({declarations: [Extends]})', + }).then((mainNgFactory) => { expect(mainNgFactory).toBeTruthy(); }); + })); + + it('should error if the child class has no matching decorator', async(() => { + compileParentAndChild({ + parentClassDecorator: `@Component({template: ''})`, + parentModuleDecorator: '@NgModule({declarations: [Base]})', + childClassDecorator: '', + childModuleDecorator: '@NgModule({declarations: [Extends]})', + }).then(fail, (e) => { + expect(e.message).toContain( + 'Class Extends in /app/main.ts extends from a Directive in another compilation unit without duplicating the decorator. ' + + 'Please add a Directive or Component decorator to the class.'); + }); + })); + }); + + describe('Directive', () => { + it('should allow to inherit', async(() => { + compileParentAndChild({ + parentClassDecorator: `@Directive({selector: '[someDir]'})`, + parentModuleDecorator: '@NgModule({declarations: [Base]})', + childClassDecorator: `@Directive({selector: '[someDir]'})`, + childModuleDecorator: '@NgModule({declarations: [Extends]})', + }).then((mainNgFactory) => { expect(mainNgFactory).toBeTruthy(); }); + })); + + it('should error if the child class has no matching decorator', async(() => { + compileParentAndChild({ + parentClassDecorator: `@Directive({selector: '[someDir]'})`, + parentModuleDecorator: '@NgModule({declarations: [Base]})', + childClassDecorator: '', + childModuleDecorator: '@NgModule({declarations: [Extends]})', + }).then(fail, (e) => { + expect(e.message).toContain( + 'Class Extends in /app/main.ts extends from a Directive in another compilation unit without duplicating the decorator. ' + + 'Please add a Directive or Component decorator to the class.'); + }); + })); + }); + + describe('Pipe', () => { + it('should allow to inherit', async(() => { + compileParentAndChild({ + parentClassDecorator: `@Pipe({name: 'somePipe'})`, + parentModuleDecorator: '@NgModule({declarations: [Base]})', + childClassDecorator: `@Pipe({name: 'somePipe'})`, + childModuleDecorator: '@NgModule({declarations: [Extends]})', + }).then((mainNgFactory) => { expect(mainNgFactory).toBeTruthy(); }); + })); + + it('should error if the child class has no matching decorator', async(() => { + compileParentAndChild({ + parentClassDecorator: `@Pipe({name: 'somePipe'})`, + parentModuleDecorator: '@NgModule({declarations: [Base]})', + childClassDecorator: '', + childModuleDecorator: '@NgModule({declarations: [Extends]})', + }).then(fail, (e) => { + expect(e.message).toContain( + 'Class Extends in /app/main.ts extends from a Pipe in another compilation unit without duplicating the decorator. ' + + 'Please add a Pipe decorator to the class.'); + }); + })); + }); + + describe('NgModule', () => { + it('should allow to inherit', async(() => { + compileParentAndChild({ + parentClassDecorator: `@NgModule()`, + parentModuleDecorator: '', + childClassDecorator: `@NgModule()`, + childModuleDecorator: '', + }).then((mainNgFactory) => { expect(mainNgFactory).toBeTruthy(); }); + })); + + it('should error if the child class has no matching decorator', async(() => { + compileParentAndChild({ + parentClassDecorator: `@NgModule()`, + parentModuleDecorator: '', + childClassDecorator: '', + childModuleDecorator: '', + }).then(fail, (e) => { + expect(e.message).toContain( + 'Class Extends in /app/main.ts extends from a NgModule in another compilation unit without duplicating the decorator. ' + + 'Please add a NgModule decorator to the class.'); + }); + })); + }); + }); }); describe('compiler (bundled Angular)', () => { @@ -513,6 +801,11 @@ function expectNoDiagnostics(program: ts.Program) { expectNoDiagnostics(program.getSemanticDiagnostics()); } +function expectNoDiagnosticsAndEmit(program: ts.Program) { + expectNoDiagnostics(program); + program.emit(); +} + function isDTS(fileName: string): boolean { return /\.d\.ts$/.test(fileName); } @@ -544,7 +837,7 @@ function summaryCompile( function compile( host: MockCompilerHost, aotHost: AotCompilerHost, preCompile?: (program: ts.Program) => void, postCompile: (program: ts.Program) => void = expectNoDiagnostics, - options: AotCompilerOptions = {}) { + options: AotCompilerOptions = {}): Promise { const scripts = host.scriptNames.slice(0); const program = ts.createProgram(scripts, settings, host); if (preCompile) preCompile(program); diff --git a/packages/compiler/test/aot/static_reflector_spec.ts b/packages/compiler/test/aot/static_reflector_spec.ts index b20769fbf9..fd651d0a4e 100644 --- a/packages/compiler/test/aot/static_reflector_spec.ts +++ b/packages/compiler/test/aot/static_reflector_spec.ts @@ -24,9 +24,10 @@ describe('StaticReflector', () => { errorRecorder?: (error: any, fileName: string) => void, collectorOptions?: CollectorOptions) { const symbolCache = new StaticSymbolCache(); host = new MockStaticSymbolResolverHost(testData, collectorOptions); - symbolResolver = - new StaticSymbolResolver(host, symbolCache, new MockSummaryResolver([]), errorRecorder); - reflector = new StaticReflector(symbolResolver, decorators, [], errorRecorder); + const summaryResolver = new MockSummaryResolver([]); + spyOn(summaryResolver, 'isLibraryFile').and.returnValue(false); + symbolResolver = new StaticSymbolResolver(host, symbolCache, summaryResolver, errorRecorder); + reflector = new StaticReflector(summaryResolver, symbolResolver, decorators, [], errorRecorder); noContext = reflector.getStaticSymbol('', ''); } diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts index 01c4c158a8..c9b71b0f6c 100644 --- a/packages/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/packages/compiler/test/aot/static_symbol_resolver_spec.ts @@ -283,6 +283,31 @@ describe('StaticSymbolResolver', () => { ]); }); + it('should only use the arity for classes from libraries without summaries', () => { + init({ + '/test.d.ts': [{ + '__symbolic': 'module', + 'version': 3, + 'metadata': { + 'AParam': {__symbolic: 'class'}, + 'AClass': { + __symbolic: 'class', + arity: 1, + members: { + __ctor__: [{ + __symbolic: 'constructor', + parameters: [symbolCache.get('/test.d.ts', 'AParam')] + }] + } + } + } + }] + }); + + expect(symbolResolver.resolveSymbol(symbolCache.get('/test.d.ts', 'AClass')).metadata) + .toEqual({__symbolic: 'class', arity: 1}); + }); + it('should be able to trace a named export', () => { const symbol = symbolResolver .resolveSymbol(symbolResolver.getSymbolByModule( diff --git a/packages/compiler/test/aot/summary_serializer_spec.ts b/packages/compiler/test/aot/summary_serializer_spec.ts index 93250d7ad9..0dc18332dd 100644 --- a/packages/compiler/test/aot/summary_serializer_spec.ts +++ b/packages/compiler/test/aot/summary_serializer_spec.ts @@ -61,7 +61,8 @@ export function main() { metadata: { __symbolic: 'class', members: {'aMethod': {__symbolic: 'function'}}, - statics: {aStatic: true} + statics: {aStatic: true}, + decorators: ['aDecoratorData'] } } ], @@ -88,8 +89,12 @@ export function main() { }); expect(summaries[1].symbol).toBe(symbolCache.get('/tmp/some_service.d.ts', 'SomeService')); - // serialization should only keep the statics... - expect(summaries[1].metadata).toEqual({__symbolic: 'class', statics: {aStatic: true}}); + // serialization should drop class decorators + expect(summaries[1].metadata).toEqual({ + __symbolic: 'class', + members: {aMethod: {__symbolic: 'function'}}, + statics: {aStatic: true} + }); expect(summaries[1].type.type.reference) .toBe(symbolCache.get('/tmp/some_service.d.ts', 'SomeService')); }); diff --git a/packages/compiler/test/aot/test_util.ts b/packages/compiler/test/aot/test_util.ts index 85d7e6496c..12da1b67aa 100644 --- a/packages/compiler/test/aot/test_util.ts +++ b/packages/compiler/test/aot/test_util.ts @@ -177,8 +177,8 @@ export class MockCompilerHost implements ts.CompilerHost { private angularSourcePath: string|undefined; private nodeModulesPath: string|undefined; - private overrides = new Map(); - private writtenFiles = new Map(); + public overrides = new Map(); + public writtenFiles = new Map(); private sourceFiles = new Map(); private assumeExists = new Set(); private traces: string[] = []; diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index c98de00651..7066dc2bbd 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -74,6 +74,7 @@ export class DummyResourceLoader extends ResourceLoader { export class TypeScriptServiceHost implements LanguageServiceHost { private _resolver: CompileMetadataResolver; private _staticSymbolCache = new StaticSymbolCache(); + private _summaryResolver: AotSummaryResolver; private _staticSymbolResolver: StaticSymbolResolver; private _reflector: StaticReflector; private _reflectorHost: ReflectorHost; @@ -407,7 +408,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { private get staticSymbolResolver(): StaticSymbolResolver { let result = this._staticSymbolResolver; if (!result) { - const summaryResolver = new AotSummaryResolver( + this._summaryResolver = new AotSummaryResolver( { loadSummary(filePath: string) { return null; }, isSourceFile(sourceFilePath: string) { return true; }, @@ -415,7 +416,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { }, this._staticSymbolCache); result = this._staticSymbolResolver = new StaticSymbolResolver( - this.reflectorHost, this._staticSymbolCache, summaryResolver, + this.reflectorHost, this._staticSymbolCache, this._summaryResolver, (e, filePath) => this.collectError(e, filePath)); } return result; @@ -424,8 +425,9 @@ export class TypeScriptServiceHost implements LanguageServiceHost { private get reflector(): StaticReflector { let result = this._reflector; if (!result) { + const ssr = this.staticSymbolResolver; result = this._reflector = new StaticReflector( - this.staticSymbolResolver, [], [], (e, filePath) => this.collectError(e, filePath)); + this._summaryResolver, ssr, [], [], (e, filePath) => this.collectError(e, filePath)); } return result; }