Skip to content

Commit

Permalink
perf(core): do not recursive into modules that have already been regi…
Browse files Browse the repository at this point in the history
…stered

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
  • Loading branch information
JoostK committed Oct 30, 2020
1 parent cbc0907 commit 4e57dee
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 16 deletions.
34 changes: 20 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,25 @@ 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 {
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);
}
}
}
}

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 4e57dee

Please sign in to comment.