diff --git a/packages/core/src/linker/ng_module_factory_registration.ts b/packages/core/src/linker/ng_module_factory_registration.ts index e63314323f..60ed6d603d 100644 --- a/packages/core/src/linker/ng_module_factory_registration.ts +++ b/packages/core/src/linker/ng_module_factory_registration.ts @@ -37,10 +37,21 @@ function assertSameOrNotExisting(id: string, type: Type| null, incoming: Ty } } -export function registerNgModuleType(id: string, ngModuleType: NgModuleType) { - const existing = modules.get(id) as NgModuleType | null; - assertSameOrNotExisting(id, existing, ngModuleType); - modules.set(id, ngModuleType); +export function registerNgModuleType(ngModuleType: NgModuleType) { + if (ngModuleType.ngModuleDef.id !== null) { + const id = ngModuleType.ngModuleDef.id; + const existing = modules.get(id) as NgModuleType | null; + assertSameOrNotExisting(id, existing, ngModuleType); + modules.set(id, ngModuleType); + } + + let imports = ngModuleType.ngModuleDef.imports; + if (imports instanceof Function) { + imports = imports(); + } + if (imports) { + imports.forEach((i: NgModuleType) => registerNgModuleType(i)); + } } export function clearModulesForTest(): void { diff --git a/packages/core/src/render3/ng_module_ref.ts b/packages/core/src/render3/ng_module_ref.ts index 96b88d10ae..f46b63e0b6 100644 --- a/packages/core/src/render3/ng_module_ref.ts +++ b/packages/core/src/render3/ng_module_ref.ts @@ -92,14 +92,39 @@ export class NgModuleRef extends viewEngine_NgModuleRef implements Interna } export class NgModuleFactory extends viewEngine_NgModuleFactory { - constructor(public moduleType: Type) { super(); } + constructor(public moduleType: Type) { + super(); + + const ngModuleDef = getNgModuleDef(moduleType); + if (ngModuleDef !== null) { + // Register the NgModule with Angular's module registry. The location (and hence timing) of + // this call is critical to ensure this works correctly (modules get registered when expected) + // without bloating bundles (modules are registered when otherwise not referenced). + // + // In View Engine, registration occurs in the .ngfactory.js file as a side effect. This has + // several practical consequences: + // + // - If an .ngfactory file is not imported from, the module won't be registered (and can be + // tree shaken). + // - If an .ngfactory file is imported from, the module will be registered even if an instance + // is not actually created (via `create` below). + // - Since an .ngfactory file in View Engine references the .ngfactory files of the NgModule's + // imports, + // + // In Ivy, things are a bit different. .ngfactory files still exist for compatibility, but are + // not a required API to use - there are other ways to obtain an NgModuleFactory for a given + // NgModule. Thus, relying on a side effect in the .ngfactory file is not sufficient. Instead, + // the side effect of registration is added here, in the constructor of NgModuleFactory, + // ensuring no matter how a factory is created, the module is registered correctly. + // + // An alternative would be to include the registration side effect inline following the actual + // NgModule definition. This also has the correct timing, but breaks tree-shaking - modules + // will be registered and retained even if they're otherwise never referenced. + registerNgModuleType(moduleType as NgModuleType); + } + } create(parentInjector: Injector|null): viewEngine_NgModuleRef { - const moduleType = this.moduleType; - const moduleRef = new NgModuleRef(moduleType, parentInjector); - const ngModuleDef = getNgModuleDef(moduleType); - ngModuleDef && ngModuleDef.id && - registerNgModuleType(ngModuleDef.id, moduleType as NgModuleType); - return moduleRef; + return new NgModuleRef(this.moduleType, parentInjector); } } diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 54ce976a3a..ee6eee8241 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ANALYZE_FOR_ENTRY_COMPONENTS, CUSTOM_ELEMENTS_SCHEMA, ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, HostBinding, Inject, Injectable, InjectionToken, Injector, Input, NgModule, NgModuleRef, Optional, Pipe, Provider, Self, Type, forwardRef, getModuleFactory, ɵivyEnabled as ivyEnabled} from '@angular/core'; +import {ANALYZE_FOR_ENTRY_COMPONENTS, CUSTOM_ELEMENTS_SCHEMA, ChangeDetectorRef, Compiler, Component, ComponentFactoryResolver, Directive, HostBinding, Inject, Injectable, InjectionToken, Injector, Input, NgModule, NgModuleRef, Optional, Pipe, Provider, Self, Type, forwardRef, getModuleFactory, ɵivyEnabled as ivyEnabled, ɵɵdefineNgModule as defineNgModule} from '@angular/core'; import {Console} from '@angular/core/src/console'; import {ɵɵInjectableDef, ɵɵdefineInjectable} from '@angular/core/src/di/interface/defs'; import {getNgModuleDef} from '@angular/core/src/render3/definition'; @@ -339,6 +339,28 @@ function declareTests(config?: {useJit: boolean}) { } }).not.toThrow(); }); + + onlyInIvy('VE does not allow use of NgModuleFactory without importing the .ngfactory') + .it('should register a module even if not importing the .ngfactory file or calling create()', + () => { + class ChildModule { + static ngModuleDef = defineNgModule({ + type: ChildModule, + id: 'child', + }); + } + + class Module { + static ngModuleDef = defineNgModule({ + type: Module, + id: 'test', + imports: [ChildModule], + }); + } + + createModuleFactory(ChildModule); + expect(getModuleFactory('child')).toBeAnInstanceOf(NgModuleFactory); + }); }); describe('entryComponents', () => {