perf(core): do not recurse into modules that have already been registered (#39514)

JoostK · josephperrott · commit 812355cc3feb · 2020-11-02T07:51:19.000-08:00
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
diff --git 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,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);
+      }
+    }
   }
 }
 
diff --git 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<any> = {bootstrap: []} as any;
+           static ɵmod = ɵɵdefineNgModule({type: MyAppModule});
          }
          const myAppModuleFactory = new NgModuleFactory(MyAppModule);
          const ngModuleRef = myAppModuleFactory.create(null);
