From 835618c67887ed323a3ce7b1b2178397f02b62eb Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sun, 23 Feb 2020 11:32:59 +0100 Subject: [PATCH] fix(ivy): error when accessing NgModuleRef.componentFactoryResolver in constructor (#35637) Currently we resolve the `NgModuleRef.componentFactoryResolver` by going through the injector, but the problem is that `ComponentFactoryResolver` has a dependency on `NgModuleRef`, which means that if the module that's attached to the ref tries to inject `ComponentFactoryResolver` in its constructor, we'll create a circular dependency which throws at runtime. These changes resolve the issue by creating the `ComponentFactoryResolver` manually ahead of time without going through the injector. We can do this safely, because the only dependency for the resolver is the current module ref which is providing it. Aside from fixing the issue, another advantage to this approach is that it should reduce the amount of generated JS, because it removes a getter and a provider definitio. Fixes #35580. PR Close #35637 --- packages/core/src/render3/ng_module_ref.ts | 38 ++++++++----------- .../core/test/acceptance/ng_module_spec.ts | 23 ++++++++++- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/packages/core/src/render3/ng_module_ref.ts b/packages/core/src/render3/ng_module_ref.ts index a5b6e4f023..9544f3c2eb 100644 --- a/packages/core/src/render3/ng_module_ref.ts +++ b/packages/core/src/render3/ng_module_ref.ts @@ -9,7 +9,6 @@ import {Injector} from '../di/injector'; import {INJECTOR} from '../di/injector_compatibility'; import {InjectFlags} from '../di/interface/injector'; -import {StaticProvider} from '../di/interface/provider'; import {R3Injector, createInjector} from '../di/r3_injector'; import {Type} from '../interface/type'; import {ComponentFactoryResolver as viewEngine_ComponentFactoryResolver} from '../linker/component_factory_resolver'; @@ -26,12 +25,6 @@ import {maybeUnwrapFn} from './util/misc_utils'; export interface NgModuleType extends Type { ɵmod: NgModuleDef; } -const COMPONENT_FACTORY_RESOLVER: StaticProvider = { - provide: viewEngine_ComponentFactoryResolver, - useClass: ComponentFactoryResolver, - deps: [viewEngine_NgModuleRef], -}; - export class NgModuleRef extends viewEngine_NgModuleRef implements InternalNgModuleRef { // tslint:disable-next-line:require-internal-with-underscore _bootstrapComponents: Type[] = []; @@ -41,6 +34,14 @@ export class NgModuleRef extends viewEngine_NgModuleRef implements Interna instance: T; destroyCbs: (() => void)[]|null = []; + // When bootstrapping a module we have a dependency graph that looks like this: + // ApplicationRef -> ComponentFactoryResolver -> NgModuleRef. The problem is that if the + // module being resolved tries to inject the ComponentFactoryResolver, it'll create a + // circular dependency which will result in a runtime error, because the injector doesn't + // exist yet. We work around the issue by creating the ComponentFactoryResolver ourselves + // and providing it, rather than letting the injector resolve it. + readonly componentFactoryResolver: ComponentFactoryResolver = new ComponentFactoryResolver(this); + constructor(ngModuleType: Type, public _parent: Injector|null) { super(); const ngModuleDef = getNgModuleDef(ngModuleType); @@ -49,20 +50,15 @@ export class NgModuleRef extends viewEngine_NgModuleRef implements Interna `NgModule '${stringify(ngModuleType)}' is not a subtype of 'NgModuleType'.`); const ngLocaleIdDef = getNgLocaleIdDef(ngModuleType); - if (ngLocaleIdDef) { - setLocaleId(ngLocaleIdDef); - } - + ngLocaleIdDef && setLocaleId(ngLocaleIdDef); this._bootstrapComponents = maybeUnwrapFn(ngModuleDef !.bootstrap); - const additionalProviders: StaticProvider[] = [ - { - provide: viewEngine_NgModuleRef, - useValue: this, - }, - COMPONENT_FACTORY_RESOLVER - ]; this._r3Injector = createInjector( - ngModuleType, _parent, additionalProviders, stringify(ngModuleType)) as R3Injector; + ngModuleType, _parent, + [ + {provide: viewEngine_NgModuleRef, useValue: this}, + {provide: viewEngine_ComponentFactoryResolver, useValue: this.componentFactoryResolver} + ], + stringify(ngModuleType)) as R3Injector; this.instance = this.get(ngModuleType); } @@ -74,10 +70,6 @@ export class NgModuleRef extends viewEngine_NgModuleRef implements Interna return this._r3Injector.get(token, notFoundValue, injectFlags); } - get componentFactoryResolver(): viewEngine_ComponentFactoryResolver { - return this.get(viewEngine_ComponentFactoryResolver); - } - destroy(): void { ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed'); const injector = this._r3Injector; diff --git a/packages/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index ec9c5be0d8..105d6d5c30 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, Injectable, NO_ERRORS_SCHEMA, NgModule, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineInjector as defineInjector, ɵɵdefineNgModule as defineNgModule, ɵɵelement as element} from '@angular/core'; +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 {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {modifiedInIvy, onlyInIvy} from '@angular/private/testing'; @@ -474,4 +474,25 @@ describe('NgModule', () => { }); }); + + it('should be able to use ComponentFactoryResolver from the NgModuleRef inside the module constructor', + () => { + let factory: ComponentFactory; + + @NgModule({ + declarations: [TestCmp], + exports: [TestCmp], + entryComponents: [TestCmp] // Only necessary for ViewEngine + }) + class MyModule { + constructor(ngModuleRef: NgModuleRef) { + factory = ngModuleRef.componentFactoryResolver.resolveComponentFactory(TestCmp); + } + } + + TestBed.configureTestingModule({imports: [MyModule]}); + TestBed.createComponent(TestCmp); + expect(factory !.componentType).toBe(TestCmp); + }); + });