From 1f8a243b671cb1ad3ba68a2fa1ebd7bb72d15fc0 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 2 Mar 2020 20:23:50 +0100 Subject: [PATCH] fix(core): unable to NgModuleRef.injector in module constructor (#35731) This is a follow up to #35637 which resolved a similar issue for `ComponentFactoryResolver`, but not the root cause. When a `NgModuleRef` is created, it instantiates an `Injector` internally which in turn resolves all of injector types. This can result in a circular call that results in an error, because the module is one of the injector types being resolved. These changes work around the issue by allowing the constructor to run before resolving the injector types. Fixes #35677. Fixes #35639. PR Close #35731 --- packages/core/src/di/r3_injector.ts | 23 ++++++++++---- packages/core/src/render3/ng_module_ref.ts | 9 ++++-- .../core/test/acceptance/ng_module_spec.ts | 30 +++++++++++++++---- .../injection/bundle.golden_symbols.json | 5 +++- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/packages/core/src/di/r3_injector.ts b/packages/core/src/di/r3_injector.ts index cd6ccb6071..10fc23852c 100644 --- a/packages/core/src/di/r3_injector.ts +++ b/packages/core/src/di/r3_injector.ts @@ -78,8 +78,21 @@ interface Record { export function createInjector( defType: /* InjectorType */ any, parent: Injector | null = null, additionalProviders: StaticProvider[] | null = null, name?: string): Injector { - parent = parent || getNullInjector(); - return new R3Injector(defType, additionalProviders, parent, name); + const injector = + createInjectorWithoutInjectorInstances(defType, parent, additionalProviders, name); + injector._resolveInjectorDefTypes(); + return injector; +} + +/** + * Creates a new injector without eagerly resolving its injector types. Can be used in places + * where resolving the injector types immediately can lead to an infinite loop. The injector types + * should be resolved at a later point by calling `_resolveInjectorDefTypes`. + */ +export function createInjectorWithoutInjectorInstances( + defType: /* InjectorType */ any, parent: Injector | null = null, + additionalProviders: StaticProvider[] | null = null, name?: string): R3Injector { + return new R3Injector(defType, additionalProviders, parent || getNullInjector(), name); } export class R3Injector { @@ -136,9 +149,6 @@ export class R3Injector { const record = this.records.get(INJECTOR_SCOPE); this.scope = record != null ? record.value : null; - // Eagerly instantiate the InjectorType classes themselves. - this.injectorDefTypes.forEach(defType => this.get(defType)); - // Source name, used for debugging this.source = source || (typeof def === 'object' ? null : stringify(def)); } @@ -224,6 +234,9 @@ export class R3Injector { } } + /** @internal */ + _resolveInjectorDefTypes() { this.injectorDefTypes.forEach(defType => this.get(defType)); } + toString() { const tokens = [], records = this.records; records.forEach((v, token) => tokens.push(stringify(token))); diff --git a/packages/core/src/render3/ng_module_ref.ts b/packages/core/src/render3/ng_module_ref.ts index 9544f3c2eb..e3c3326082 100644 --- a/packages/core/src/render3/ng_module_ref.ts +++ b/packages/core/src/render3/ng_module_ref.ts @@ -9,7 +9,7 @@ import {Injector} from '../di/injector'; import {INJECTOR} from '../di/injector_compatibility'; import {InjectFlags} from '../di/interface/injector'; -import {R3Injector, createInjector} from '../di/r3_injector'; +import {R3Injector, createInjectorWithoutInjectorInstances} from '../di/r3_injector'; import {Type} from '../interface/type'; import {ComponentFactoryResolver as viewEngine_ComponentFactoryResolver} from '../linker/component_factory_resolver'; import {InternalNgModuleRef, NgModuleFactory as viewEngine_NgModuleFactory, NgModuleRef as viewEngine_NgModuleRef} from '../linker/ng_module_factory'; @@ -52,13 +52,18 @@ export class NgModuleRef extends viewEngine_NgModuleRef implements Interna const ngLocaleIdDef = getNgLocaleIdDef(ngModuleType); ngLocaleIdDef && setLocaleId(ngLocaleIdDef); this._bootstrapComponents = maybeUnwrapFn(ngModuleDef !.bootstrap); - this._r3Injector = createInjector( + this._r3Injector = createInjectorWithoutInjectorInstances( ngModuleType, _parent, [ {provide: viewEngine_NgModuleRef, useValue: this}, {provide: viewEngine_ComponentFactoryResolver, useValue: this.componentFactoryResolver} ], stringify(ngModuleType)) as R3Injector; + + // We need to resolve the injector types separately from the injector creation, because + // the module might be trying to use this ref in its contructor for DI which will cause a + // circular error that will eventually error out, because the injector isn't created yet. + this._r3Injector._resolveInjectorDefTypes(); this.instance = this.get(ngModuleType); } diff --git a/packages/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index 105d6d5c30..e23031c32e 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -7,7 +7,7 @@ */ import {CommonModule} from '@angular/common'; -import {CUSTOM_ELEMENTS_SCHEMA, Component, ComponentFactory, Injectable, NO_ERRORS_SCHEMA, NgModule, NgModuleRef, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineInjector as defineInjector, ɵɵdefineNgModule as defineNgModule, ɵɵelement as element} from '@angular/core'; +import {CUSTOM_ELEMENTS_SCHEMA, Component, Injectable, InjectionToken, NO_ERRORS_SCHEMA, NgModule, NgModuleRef, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineInjector as defineInjector, ɵɵdefineNgModule as defineNgModule, ɵɵelement as element} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {modifiedInIvy, onlyInIvy} from '@angular/private/testing'; @@ -475,9 +475,28 @@ describe('NgModule', () => { }); - it('should be able to use ComponentFactoryResolver from the NgModuleRef inside the module constructor', + it('should be able to use DI through the NgModuleRef inside the module constructor', () => { + const token = new InjectionToken('token'); + let value: string|undefined; + + @NgModule({ + imports: [CommonModule], + providers: [{provide: token, useValue: 'foo'}], + }) + class TestModule { + constructor(ngRef: NgModuleRef) { value = ngRef.injector.get(token); } + } + + TestBed.configureTestingModule({imports: [TestModule], declarations: [TestCmp]}); + const fixture = TestBed.createComponent(TestCmp); + fixture.detectChanges(); + + expect(value).toBe('foo'); + }); + + it('should be able to create a component through the ComponentFactoryResolver of an NgModuleRef in a module constructor', () => { - let factory: ComponentFactory; + let componentInstance: TestCmp|undefined; @NgModule({ declarations: [TestCmp], @@ -486,13 +505,14 @@ describe('NgModule', () => { }) class MyModule { constructor(ngModuleRef: NgModuleRef) { - factory = ngModuleRef.componentFactoryResolver.resolveComponentFactory(TestCmp); + const factory = ngModuleRef.componentFactoryResolver.resolveComponentFactory(TestCmp); + componentInstance = factory.create(ngModuleRef.injector).instance; } } TestBed.configureTestingModule({imports: [MyModule]}); TestBed.createComponent(TestCmp); - expect(factory !.componentType).toBe(TestCmp); + expect(componentInstance).toBeAnInstanceOf(TestCmp); }); }); diff --git a/packages/core/test/bundling/injection/bundle.golden_symbols.json b/packages/core/test/bundling/injection/bundle.golden_symbols.json index 58d5050179..125db87f31 100644 --- a/packages/core/test/bundling/injection/bundle.golden_symbols.json +++ b/packages/core/test/bundling/injection/bundle.golden_symbols.json @@ -107,6 +107,9 @@ { "name": "createInjector" }, + { + "name": "createInjectorWithoutInjectorInstances" + }, { "name": "deepForEach" }, @@ -224,4 +227,4 @@ { "name": "noSideEffects" } -] \ No newline at end of file +]