Skip to content

Commit

Permalink
perf(core): do not recurse into modules that have already been regist…
Browse files Browse the repository at this point in the history
…ered (#39514)

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

PR Close #39514
  • Loading branch information
JoostK authored and josephperrott committed Nov 2, 2020
1 parent 71d0063 commit 812355c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 16 deletions.
36 changes: 22 additions & 14 deletions packages/core/src/linker/ng_module_factory_registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -39,20 +40,27 @@ function assertSameOrNotExisting(id: string, type: Type<any>|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<NgModuleType>();
recurse(ngModuleType);
function recurse(ngModuleType: NgModuleType): void {
// 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;
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);
}
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/render3/providers_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1092,7 +1092,7 @@ describe('providers', () => {
{provide: String, useValue: 'From module injector'}
]
});
static ɵmod: NgModuleDef<any> = {bootstrap: []} as any;
static ɵmod = ɵɵdefineNgModule({type: MyAppModule});
}
const myAppModuleFactory = new NgModuleFactory(MyAppModule);
const ngModuleRef = myAppModuleFactory.create(null);
Expand Down

0 comments on commit 812355c

Please sign in to comment.