refactor(core): NgModuleRef should not implement EnvironmentInjector interface (#46896)

AndrewKushnir · AndrewKushnir · commit 7a6509bdc1b1 · 2022-07-20T08:49:15.000-07:00
This commit refactors the `NgModuleRef` implementation to drop functions required by the `EnvironmentInjector` interface. Previously the idea was that the `NgModuleRef` can act as an Injector to facilitate easier transition to standalone. However, from the mental model perspective, the `NgModuleRef` has the `injector` field, which is the correct injector reference and can be used is needed as an `EnvironmentInjector`. PR Close #46896
diff --git a/packages/core/src/render3/ng_module_ref.ts b/packages/core/src/render3/ng_module_ref.ts
@@ -44,13 +44,11 @@ export function createNgModule<T>(
  * @deprecated Use `createNgModule` instead.
  */
 export const createNgModuleRef = createNgModule;
-export class NgModuleRef<T> extends viewEngine_NgModuleRef<T> implements InternalNgModuleRef<T>,
-                                                                         EnvironmentInjector {
+export class NgModuleRef<T> extends viewEngine_NgModuleRef<T> implements InternalNgModuleRef<T> {
   // tslint:disable-next-line:require-internal-with-underscore
   _bootstrapComponents: Type<any>[] = [];
   // tslint:disable-next-line:require-internal-with-underscore
   _r3Injector: R3Injector;
-  override injector: EnvironmentInjector = this;
   override instance: T;
   destroyCbs: (() => void)[]|null = [];
 
@@ -86,19 +84,11 @@ export class NgModuleRef<T> extends viewEngine_NgModuleRef<T> implements Interna
     // the module might be trying to use this ref in its constructor for DI which will cause a
     // circular error that will eventually error out, because the injector isn't created yet.
     this._r3Injector.resolveInjectorInitializers();
-    this.instance = this.get(ngModuleType);
+    this.instance = this._r3Injector.get(ngModuleType);
   }
 
-  get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND,
-      injectFlags: InjectFlags = InjectFlags.Default): any {
-    if (token === Injector || token === viewEngine_NgModuleRef || token === INJECTOR) {
-      return this;
-    }
-    return this._r3Injector.get(token, notFoundValue, injectFlags);
-  }
-
-  runInContext<ReturnT>(fn: () => ReturnT): ReturnT {
-    return this.injector.runInContext(fn);
+  override get injector(): EnvironmentInjector {
+    return this._r3Injector;
   }
 
   override destroy(): void {
diff --git a/packages/core/test/acceptance/environment_injector_spec.ts b/packages/core/test/acceptance/environment_injector_spec.ts
@@ -141,6 +141,19 @@ describe('environment injector', () => {
       expect(returnValue).toBe(3);
     });
 
+    it('should work with an NgModuleRef injector', () => {
+      const ref = TestBed.inject(NgModuleRef);
+      const returnValue = ref.injector.runInContext(() => 3);
+      expect(returnValue).toBe(3);
+    });
+
+    it('should return correct injector reference', () => {
+      const ngModuleRef = TestBed.inject(NgModuleRef);
+      const ref1 = ngModuleRef.injector.runInContext(() => inject(Injector));
+      const ref2 = ngModuleRef.injector.get(Injector);
+      expect(ref1).toBe(ref2);
+    });
+
     it('should make inject() available', () => {
       const TOKEN = new InjectionToken<string>('TOKEN');
       const injector = createEnvironmentInjector(
diff --git a/packages/router/test/router_preloader.spec.ts b/packages/router/test/router_preloader.spec.ts
@@ -111,12 +111,12 @@ describe('RouterPreloader', () => {
              const injector: any = getLoadedInjector(c[0]);
              const loadedRoutes: Route[] = getLoadedRoutes(c[0])!;
              expect(loadedRoutes[0].path).toEqual('LoadedModule1');
-             expect(injector._parent).toBe((testModule as any)._r3Injector);
+             expect(injector.parent).toBe((testModule as any)._r3Injector);
 
              const injector2: any = getLoadedInjector(loadedRoutes[0]);
              const loadedRoutes2: Route[] = getLoadedRoutes(loadedRoutes[0])!;
              expect(loadedRoutes2[0].path).toEqual('LoadedModule2');
-             expect(injector2._parent).toBe(injector);
+             expect(injector2.parent).toBe(injector);
 
              expect(events.map(e => e.toString())).toEqual([
                'RouteConfigLoadStart(path: lazy)',
@@ -213,11 +213,11 @@ describe('RouterPreloader', () => {
 
              const injector = getLoadedInjector(c[0]) as any;
              const loadedRoutes = getLoadedRoutes(c[0])!;
-             expect(injector._parent).toBe((testModule as any)._r3Injector);
+             expect(injector.parent).toBe((testModule as any)._r3Injector);
 
              const loadedRoutes2: Route[] = getLoadedRoutes(loadedRoutes[0])!;
              const injector3: any = getLoadedInjector(loadedRoutes2[0]);
-             expect(injector3._parent).toBe(module2.injector);
+             expect(injector3.parent).toBe(module2.injector);
            })));
   });
 
