From dfaf45026352e29a0cb29ce2ed7071ab03b81af4 Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 30 Oct 2020 20:31:47 +0100 Subject: [PATCH 1/2] perf(core): do not recurse into modules that have already been registered When registering an NgModule based on its id, all transitively imported NgModules are also registered. This commit introduces a visited set to avoid traversing into NgModules that are reachable from multiple import paths multiple times. Fixes #39487 --- .../linker/ng_module_factory_registration.ts | 34 +++++++++++-------- packages/core/test/render3/providers_spec.ts | 4 +-- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/core/src/linker/ng_module_factory_registration.ts b/packages/core/src/linker/ng_module_factory_registration.ts index 86a4f962d84d..190feda34266 100644 --- a/packages/core/src/linker/ng_module_factory_registration.ts +++ b/packages/core/src/linker/ng_module_factory_registration.ts @@ -8,8 +8,9 @@ import {Type} from '../interface/type'; -import {autoRegisterModuleById} from '../render3/definition'; +import {autoRegisterModuleById, getNgModuleDef} from '../render3/definition'; import {NgModuleType} from '../render3/ng_module_ref'; +import {maybeUnwrapFn} from '../render3/util/misc_utils'; import {stringify} from '../util/stringify'; import {NgModuleFactory} from './ng_module_factory'; @@ -39,20 +40,25 @@ function assertSameOrNotExisting(id: string, type: Type|null, incoming: Typ } } -export function registerNgModuleType(ngModuleType: NgModuleType) { - if (ngModuleType.ɵmod.id !== null) { - const id = ngModuleType.ɵmod.id; - const existing = modules.get(id) as NgModuleType | null; - assertSameOrNotExisting(id, existing, ngModuleType); - modules.set(id, ngModuleType); - } +export function registerNgModuleType(ngModuleType: NgModuleType): void { + const visited = new Set(); + recurse(ngModuleType); + function recurse(ngModuleType: NgModuleType): void { + const def = getNgModuleDef(ngModuleType, true); + const id = def.id; + if (id !== null) { + const existing = modules.get(id) as NgModuleType | null; + assertSameOrNotExisting(id, existing, ngModuleType); + modules.set(id, ngModuleType); + } - let imports = ngModuleType.ɵmod.imports; - if (imports instanceof Function) { - imports = imports(); - } - if (imports) { - imports.forEach(i => registerNgModuleType(i as NgModuleType)); + const imports = maybeUnwrapFn(def.imports) as NgModuleType[]; + for (const i of imports) { + if (!visited.has(i)) { + visited.add(i); + recurse(i); + } + } } } diff --git a/packages/core/test/render3/providers_spec.ts b/packages/core/test/render3/providers_spec.ts index b75bf504ab37..5d2f6f507797 100644 --- a/packages/core/test/render3/providers_spec.ts +++ b/packages/core/test/render3/providers_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component as _Component, ComponentFactoryResolver, ElementRef, Injectable as _Injectable, InjectFlags, InjectionToken, InjectorType, Provider, RendererFactory2, Type, ViewContainerRef, ɵNgModuleDef as NgModuleDef, ɵɵdefineInjectable, ɵɵdefineInjector, ɵɵinject} from '../../src/core'; +import {Component as _Component, ComponentFactoryResolver, ElementRef, Injectable as _Injectable, InjectFlags, InjectionToken, InjectorType, Provider, RendererFactory2, Type, ViewContainerRef, ɵɵdefineInjectable, ɵɵdefineInjector, ɵɵdefineNgModule, ɵɵinject} from '../../src/core'; import {forwardRef} from '../../src/di/forward_ref'; import {createInjector} from '../../src/di/r3_injector'; import {injectComponentFactoryResolver, ɵɵdefineComponent, ɵɵdefineDirective, ɵɵdirectiveInject, ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵgetInheritedFactory, ɵɵProvidersFeature, ɵɵtext, ɵɵtextInterpolate1} from '../../src/render3/index'; @@ -1092,7 +1092,7 @@ describe('providers', () => { {provide: String, useValue: 'From module injector'} ] }); - static ɵmod: NgModuleDef = {bootstrap: []} as any; + static ɵmod = ɵɵdefineNgModule({type: MyAppModule}); } const myAppModuleFactory = new NgModuleFactory(MyAppModule); const ngModuleRef = myAppModuleFactory.create(null); From 2b052351ec4265c55a0399d82d73d4a9829c095f Mon Sep 17 00:00:00 2001 From: JoostK Date: Fri, 30 Oct 2020 22:08:35 +0100 Subject: [PATCH 2/2] fixup! perf(core): do not recurse into modules that have already been registered --- packages/core/src/linker/ng_module_factory_registration.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/linker/ng_module_factory_registration.ts b/packages/core/src/linker/ng_module_factory_registration.ts index 190feda34266..b47e635778c8 100644 --- a/packages/core/src/linker/ng_module_factory_registration.ts +++ b/packages/core/src/linker/ng_module_factory_registration.ts @@ -44,7 +44,9 @@ export function registerNgModuleType(ngModuleType: NgModuleType): void { const visited = new Set(); recurse(ngModuleType); function recurse(ngModuleType: NgModuleType): void { - const def = getNgModuleDef(ngModuleType, true); + // The imports array of an NgModule must refer to other NgModules, + // so an error is thrown if no module definition is available. + const def = getNgModuleDef(ngModuleType, /* throwNotFound */ true); const id = def.id; if (id !== null) { const existing = modules.get(id) as NgModuleType | null;