diff --git a/packages/compiler/src/ng_module_compiler.ts b/packages/compiler/src/ng_module_compiler.ts index beff9b3fe02c6..94d25a5a217b3 100644 --- a/packages/compiler/src/ng_module_compiler.ts +++ b/packages/compiler/src/ng_module_compiler.ts @@ -84,6 +84,7 @@ class _InjectorBuilder implements ClassBuilder { getters: o.ClassGetter[] = []; methods: o.ClassMethod[] = []; ctorStmts: o.Statement[] = []; + private _lazyProps = new Map(); private _tokens: CompileTokenMetadata[] = []; private _instances = new Map(); private _createStmts: o.Statement[] = []; @@ -103,7 +104,11 @@ class _InjectorBuilder implements ClassBuilder { propName, resolvedProvider, providerValueExpressions, resolvedProvider.multiProvider, resolvedProvider.eager); if (resolvedProvider.lifecycleHooks.indexOf(ɵLifecycleHooks.OnDestroy) !== -1) { - this._destroyStmts.push(instance.callMethod('ngOnDestroy', []).toStmt()); + let callNgOnDestroy: o.Expression = instance.callMethod('ngOnDestroy', []); + if (!resolvedProvider.eager) { + callNgOnDestroy = this._lazyProps.get(instance.name).and(callNgOnDestroy); + } + this._destroyStmts.push(callNgOnDestroy.toStmt()); } this._tokens.push(resolvedProvider.token); this._instances.set(tokenReference(resolvedProvider.token), instance); @@ -173,7 +178,7 @@ class _InjectorBuilder implements ClassBuilder { private _createProviderProperty( propName: string, provider: ProviderAst, providerValueExpressions: o.Expression[], - isMulti: boolean, isEager: boolean): o.Expression { + isMulti: boolean, isEager: boolean): o.ReadPropExpr { let resolvedProviderValueExpr: o.Expression; let type: o.Type; if (isMulti) { @@ -190,16 +195,17 @@ class _InjectorBuilder implements ClassBuilder { this.fields.push(new o.ClassField(propName, type)); this._createStmts.push(o.THIS_EXPR.prop(propName).set(resolvedProviderValueExpr).toStmt()); } else { - const internalField = `_${propName}`; - this.fields.push(new o.ClassField(internalField, type)); + const internalFieldProp = o.THIS_EXPR.prop(`_${propName}`); + this.fields.push(new o.ClassField(internalFieldProp.name, type)); // Note: Equals is important for JS so that it also checks the undefined case! const getterStmts = [ new o.IfStmt( - o.THIS_EXPR.prop(internalField).isBlank(), - [o.THIS_EXPR.prop(internalField).set(resolvedProviderValueExpr).toStmt()]), - new o.ReturnStatement(o.THIS_EXPR.prop(internalField)) + internalFieldProp.isBlank(), + [internalFieldProp.set(resolvedProviderValueExpr).toStmt()]), + new o.ReturnStatement(internalFieldProp) ]; this.getters.push(new o.ClassGetter(propName, getterStmts, type)); + this._lazyProps.set(propName, internalFieldProp); } return o.THIS_EXPR.prop(propName); } diff --git a/packages/compiler/src/provider_analyzer.ts b/packages/compiler/src/provider_analyzer.ts index 133a263e70dc1..392680307a5e6 100644 --- a/packages/compiler/src/provider_analyzer.ts +++ b/packages/compiler/src/provider_analyzer.ts @@ -462,9 +462,10 @@ function _resolveProviders( (provider.token.identifier).lifecycleHooks ? (provider.token.identifier).lifecycleHooks : []; + const isUseValue = !(provider.useClass || provider.useExisting || provider.useFactory); resolvedProvider = new ProviderAst( - provider.token, provider.multi, eager || lifecycleHooks.length > 0, [provider], - providerType, lifecycleHooks, sourceSpan); + provider.token, provider.multi, eager || isUseValue, [provider], providerType, + lifecycleHooks, sourceSpan); targetProvidersByToken.set(tokenReference(provider.token), resolvedProvider); } else { if (!provider.multi) { diff --git a/packages/core/src/application_module.ts b/packages/core/src/application_module.ts index 90cd5b3ed93bc..f03e7f682d526 100644 --- a/packages/core/src/application_module.ts +++ b/packages/core/src/application_module.ts @@ -56,4 +56,6 @@ export function _initViewEngine() { ] }) export class ApplicationModule { + // Inject ApplicationRef to make it eager... + constructor(appRef: ApplicationRef) {} } diff --git a/packages/core/src/view/provider.ts b/packages/core/src/view/provider.ts index 83f1725caf432..d33fd98cbe7b2 100644 --- a/packages/core/src/view/provider.ts +++ b/packages/core/src/view/provider.ts @@ -468,6 +468,9 @@ function callElementProvidersLifecycles(view: ViewData, elDef: NodeDef, lifecycl function callProviderLifecycles(view: ViewData, index: number, lifecycles: NodeFlags) { const provider = asProviderData(view, index).instance; + if (provider === NOT_CREATED) { + return; + } Services.setCurrentNode(view, index); if (lifecycles & NodeFlags.AfterContentInit) { provider.ngAfterContentInit(); diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 54dd584860520..a6755f38dd805 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -907,6 +907,8 @@ function declareTests({useJit}: {useJit: boolean}) { @NgModule({providers: [SomeInjectable]}) class SomeModule { + // Inject SomeInjectable to make it eager... + constructor(i: SomeInjectable) {} } const moduleRef = createModule(SomeModule); @@ -915,17 +917,33 @@ function declareTests({useJit}: {useJit: boolean}) { expect(destroyed).toBe(true); }); - it('should instantiate providers with lifecycle eagerly', () => { + it('should support ngOnDestroy for lazy providers', () => { let created = false; + let destroyed = false; class SomeInjectable { constructor() { created = true; } - ngOnDestroy() {} + ngOnDestroy() { destroyed = true; } } - createInjector([SomeInjectable]); + @NgModule({providers: [SomeInjectable]}) + class SomeModule { + } + + let moduleRef = createModule(SomeModule); + expect(created).toBe(false); + expect(destroyed).toBe(false); + + // no error if the provider was not yet created + moduleRef.destroy(); + expect(created).toBe(false); + expect(destroyed).toBe(false); + moduleRef = createModule(SomeModule); + moduleRef.injector.get(SomeInjectable); expect(created).toBe(true); + moduleRef.destroy(); + expect(destroyed).toBe(true); }); }); diff --git a/packages/core/test/linker/view_injector_integration_spec.ts b/packages/core/test/linker/view_injector_integration_spec.ts index 36a46e19282e3..fb42c9d25461a 100644 --- a/packages/core/test/linker/view_injector_integration_spec.ts +++ b/packages/core/test/linker/view_injector_integration_spec.ts @@ -342,34 +342,36 @@ export function main() { expect(created).toBe(true); }); - it('should instantiate providers lazily', () => { - TestBed.configureTestingModule({declarations: [SimpleDirective]}); + it('should support ngOnDestroy for lazy providers', () => { let created = false; - TestBed.overrideDirective( - SimpleDirective, - {set: {providers: [{provide: 'service', useFactory: () => created = true}]}}); + let destroyed = false; - const el = createComponent('
'); + class SomeInjectable { + constructor() { created = true; } + ngOnDestroy() { destroyed = true; } + } - expect(created).toBe(false); + @Component({providers: [SomeInjectable], template: ''}) + class SomeComp { + } - el.children[0].injector.get('service'); + TestBed.configureTestingModule({declarations: [SomeComp]}); - expect(created).toBe(true); - }); - it('should instantiate providers with a lifecycle hook eagerly', () => { - let created = false; - class SomeInjectable { - constructor() { created = true; } - ngOnDestroy() {} - } - TestBed.configureTestingModule({declarations: [SimpleDirective]}); - TestBed.overrideDirective(SimpleDirective, {set: {providers: [SomeInjectable]}}); + let compRef = TestBed.createComponent(SomeComp).componentRef; + expect(created).toBe(false); + expect(destroyed).toBe(false); - const el = createComponent('
'); + // no error if the provider was not yet created + compRef.destroy(); + expect(created).toBe(false); + expect(destroyed).toBe(false); + compRef = TestBed.createComponent(SomeComp).componentRef; + compRef.injector.get(SomeInjectable); expect(created).toBe(true); + compRef.destroy(); + expect(destroyed).toBe(true); }); it('should instantiate view providers lazily', () => { diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 8fd8cca6ef25e..7318b31d0a1e4 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -213,7 +213,13 @@ export class TestBed implements Injector { this._imports = []; this._schemas = []; this._instantiated = false; - this._activeFixtures.forEach((fixture) => fixture.destroy()); + this._activeFixtures.forEach((fixture) => { + try { + fixture.destroy(); + } catch (e) { + console.error('Error during cleanup of component', fixture.componentInstance); + } + }); this._activeFixtures = []; } diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts index 9cd30c44b93d7..049673e430921 100644 --- a/packages/router/src/router_module.ts +++ b/packages/router/src/router_module.ts @@ -123,7 +123,8 @@ export function routerNgProbeToken() { */ @NgModule({declarations: ROUTER_DIRECTIVES, exports: ROUTER_DIRECTIVES}) export class RouterModule { - constructor(@Optional() @Inject(ROUTER_FORROOT_GUARD) guard: any) {} + // Note: We are injecting the Router so it gets created eagerly... + constructor(@Optional() @Inject(ROUTER_FORROOT_GUARD) guard: any, @Optional() router: Router) {} /** * Creates a module with all the router providers and directives. It also optionally sets up an diff --git a/tools/public_api_guard/core/typings/core.d.ts b/tools/public_api_guard/core/typings/core.d.ts index 059e5d9be710b..5a239296d8b89 100644 --- a/tools/public_api_guard/core/typings/core.d.ts +++ b/tools/public_api_guard/core/typings/core.d.ts @@ -121,6 +121,7 @@ export declare class ApplicationInitStatus { /** @experimental */ export declare class ApplicationModule { + constructor(appRef: ApplicationRef); } /** @stable */ diff --git a/tools/public_api_guard/router/typings/router.d.ts b/tools/public_api_guard/router/typings/router.d.ts index cdefc4795f613..514fbddc64999 100644 --- a/tools/public_api_guard/router/typings/router.d.ts +++ b/tools/public_api_guard/router/typings/router.d.ts @@ -308,7 +308,7 @@ export declare class RouterLinkWithHref implements OnChanges, OnDestroy { /** @stable */ export declare class RouterModule { - constructor(guard: any); + constructor(guard: any, router: Router); static forChild(routes: Routes): ModuleWithProviders; static forRoot(routes: Routes, config?: ExtraOptions): ModuleWithProviders; }