fix(ivy): set proper implementation for module injector (angular#28667)

Prior to this change we used current injector implementation for module injector, which was causing problems and produces circular dependencies in case the same token is referenced (with @SkipSelf flag) in the `deps` array. The origin of the problem was that once `directiveInject` implementation becomes active, it was used for module injector as well, thus searching deps in Component/Directive DI scope. This fix sets `injectInjectorOnly` implementation for module injector to resolve the problem.

PR Close angular#28667

Author: AndrewKushnir

packages/core/src/render3/component_ref.ts

@@ -10,6 +10,7 @@ import {ChangeDetectorRef as ViewEngine_ChangeDetectorRef} from '../change_detec
 import {InjectionToken} from '../di/injection_token';
 import {Injector} from '../di/injector';
 import {inject} from '../di/injector_compatibility';
+import {InjectFlags} from '../di/interface/injector';
 import {Type} from '../interface/type';
 import {ComponentFactory as viewEngine_ComponentFactory, ComponentRef as viewEngine_ComponentRef} from '../linker/component_factory';
 import {ComponentFactoryResolver as viewEngine_ComponentFactoryResolver} from '../linker/component_factory_resolver';
@@ -77,8 +78,8 @@ export const SCHEDULER = new InjectionToken<((fn: () => void) => void)>('SCHEDUL
 
 function createChainedInjector(rootViewInjector: Injector, moduleInjector: Injector): Injector {
   return {
-    get: <T>(token: Type<T>| InjectionToken<T>, notFoundValue?: T): T => {
-      const value = rootViewInjector.get(token, NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR);
+    get: <T>(token: Type<T>| InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): T => {
+      const value = rootViewInjector.get(token, NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR as T, flags);
 
       if (value !== NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR ||
           notFoundValue === NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR) {
@@ -90,7 +91,7 @@ function createChainedInjector(rootViewInjector: Injector, moduleInjector: Injec
         return value;
       }
 
-      return moduleInjector.get(token, notFoundValue);
+      return moduleInjector.get(token, notFoundValue, flags);
     }
   };
 }

packages/core/src/render3/di.ts

@@ -392,10 +392,18 @@ export function getOrCreateInjectable<T>(
 
   if ((flags & (InjectFlags.Self | InjectFlags.Host)) === 0) {
     const moduleInjector = lView[INJECTOR];
-    if (moduleInjector) {
-      return moduleInjector.get(token, notFoundValue, flags & InjectFlags.Optional);
-    } else {
-      return injectRootLimpMode(token, notFoundValue, flags & InjectFlags.Optional);
+    // switch to `injectInjectorOnly` implementation for module injector, since module injector
+    // should not have access to Component/Directive DI scope (that may happen through
+    // `directiveInject` implementation)
+    const previousInjectImplementation = setInjectImplementation(undefined);
+    try {
+      if (moduleInjector) {
+        return moduleInjector.get(token, notFoundValue, flags & InjectFlags.Optional);
+      } else {
+        return injectRootLimpMode(token, notFoundValue, flags & InjectFlags.Optional);
+      }
+    } finally {
+      setInjectImplementation(previousInjectImplementation);
     }
   }
   if (flags & InjectFlags.Optional) {

packages/core/test/acceptance/di_spec.ts

@@ -7,10 +7,11 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {ChangeDetectorRef, Component, Pipe, PipeTransform} from '@angular/core';
+import {ChangeDetectorRef, Component, Directive, Inject, LOCALE_ID, Optional, Pipe, PipeTransform, SkipSelf, ViewChild} from '@angular/core';
 import {ViewRef} from '@angular/core/src/render3/view_ref';
 import {TestBed} from '@angular/core/testing';
 
+
 describe('di', () => {
   describe('ChangeDetectorRef', () => {
     it('should inject host component ChangeDetectorRef into directives on templates', () => {
@@ -39,4 +40,94 @@ describe('di', () => {
       expect((pipeInstance !.cdr as ViewRef<MyApp>).context).toBe(fixture.componentInstance);
     });
   });
+
+  it('should not cause cyclic dependency if same token is requested in deps with @SkipSelf', () => {
+    @Component({
+      selector: 'my-comp',
+      template: '...',
+      providers: [{
+        provide: LOCALE_ID,
+        useFactory: () => 'ja-JP',
+        // Note: `LOCALE_ID` is also provided within APPLICATION_MODULE_PROVIDERS, so we use it here
+        // as a dep and making sure it doesn't cause cyclic dependency (since @SkipSelf is present)
+        deps: [[new Inject(LOCALE_ID), new Optional(), new SkipSelf()]]
+      }]
+    })
+    class MyComp {
+      constructor(@Inject(LOCALE_ID) public localeId: string) {}
+    }
+
+    TestBed.configureTestingModule({declarations: [MyComp]});
+    const fixture = TestBed.createComponent(MyComp);
+    fixture.detectChanges();
+    expect(fixture.componentInstance.localeId).toBe('ja-JP');
+  });
+
+  it('module-level deps should not access Component/Directive providers', () => {
+    @Component({
+      selector: 'my-comp',
+      template: '...',
+      providers: [{
+        provide: 'LOCALE_ID_DEP',  //
+        useValue: 'LOCALE_ID_DEP_VALUE'
+      }]
+    })
+    class MyComp {
+      constructor(@Inject(LOCALE_ID) public localeId: string) {}
+    }
+
+    TestBed.configureTestingModule({
+      declarations: [MyComp],
+      providers: [{
+        provide: LOCALE_ID,
+        // we expect `localeDepValue` to be undefined, since it's not provided at a module level
+        useFactory: (localeDepValue: any) => localeDepValue || 'en-GB',
+        deps: [[new Inject('LOCALE_ID_DEP'), new Optional()]]
+      }]
+    });
+    const fixture = TestBed.createComponent(MyComp);
+    fixture.detectChanges();
+    expect(fixture.componentInstance.localeId).toBe('en-GB');
+  });
+
+  it('should skip current level while retrieving tokens if @SkipSelf is defined', () => {
+    @Component({
+      selector: 'my-comp',
+      template: '...',
+      providers: [{provide: LOCALE_ID, useFactory: () => 'en-GB'}]
+    })
+    class MyComp {
+      constructor(@SkipSelf() @Inject(LOCALE_ID) public localeId: string) {}
+    }
+
+    TestBed.configureTestingModule({declarations: [MyComp]});
+    const fixture = TestBed.createComponent(MyComp);
+    fixture.detectChanges();
+    // takes `LOCALE_ID` from module injector, since we skip Component level with @SkipSelf
+    expect(fixture.componentInstance.localeId).toBe('en-US');
+  });
+
+  it('should work when injecting dependency in Directives', () => {
+    @Directive({
+      selector: '[dir]',  //
+      providers: [{provide: LOCALE_ID, useValue: 'ja-JP'}]
+    })
+    class MyDir {
+      constructor(@SkipSelf() @Inject(LOCALE_ID) public localeId: string) {}
+    }
+    @Component({
+      selector: 'my-comp',
+      template: '<div dir></div>',
+      providers: [{provide: LOCALE_ID, useValue: 'en-GB'}]
+    })
+    class MyComp {
+      @ViewChild(MyDir) myDir !: MyDir;
+      constructor(@Inject(LOCALE_ID) public localeId: string) {}
+    }
+
+    TestBed.configureTestingModule({declarations: [MyDir, MyComp, MyComp]});
+    const fixture = TestBed.createComponent(MyComp);
+    fixture.detectChanges();
+    expect(fixture.componentInstance.myDir.localeId).toBe('en-GB');
+  });
 });