fix(ivy): injecting incorrect provider when re-providing injectable with useClass

packages/core/src/di/r3_injector.ts

@@ -12,6 +12,7 @@ import {OnDestroy} from '../interface/lifecycle_hooks';
 import {Type} from '../interface/type';
 import {getFactoryDef} from '../render3/definition';
 import {throwCyclicDependencyError, throwInvalidProviderError, throwMixedMultiProviderError} from '../render3/errors';
+import {FactoryFn} from '../render3/interfaces/definition';
 import {deepForEach, newArray} from '../util/array_utils';
 import {stringify} from '../util/stringify';
 import {resolveForwardRef} from './forward_ref';
@@ -407,7 +408,7 @@ export class R3Injector {
   }
 }
 
-function injectableDefOrInjectorDefFactory(token: Type<any>| InjectionToken<any>): () => any {
+function injectableDefOrInjectorDefFactory(token: Type<any>| InjectionToken<any>): FactoryFn<any> {
   // Most tokens will have an injectable def directly on them, which specifies a factory directly.
   const injectableDef = getInjectableDef(token);
   const factory = injectableDef !== null ? injectableDef.factory : getFactoryDef(token);
@@ -478,7 +479,8 @@ export function providerToFactory(
     provider: SingleProvider, ngModuleType?: InjectorType<any>, providers?: any[]): () => any {
   let factory: (() => any)|undefined = undefined;
   if (isTypeProvider(provider)) {
-    return injectableDefOrInjectorDefFactory(resolveForwardRef(provider));
+    const unwrappedProvider = resolveForwardRef(provider);
+    return getFactoryDef(unwrappedProvider) || injectableDefOrInjectorDefFactory(unwrappedProvider);
   } else {
     if (isValueProvider(provider)) {
       factory = () => resolveForwardRef(provider.useValue);
@@ -496,7 +498,7 @@ export function providerToFactory(
       if (hasDeps(provider)) {
         factory = () => new (classRef)(...injectArgs(provider.deps));
       } else {
-        return injectableDefOrInjectorDefFactory(classRef);
+        return getFactoryDef(classRef) || injectableDefOrInjectorDefFactory(classRef);
       }
     }
   }

packages/core/test/acceptance/di_spec.ts

@@ -1066,6 +1066,112 @@ describe('di', () => {
 
   });
 
+  describe('service injection with useClass', () => {
+    @Injectable({providedIn: 'root'})
+    class BarServiceDep {
+      name = 'BarServiceDep';
+    }
+
+    @Injectable({providedIn: 'root'})
+    class BarService {
+      constructor(public dep: BarServiceDep) {}
+      getMessage() { return 'bar'; }
+    }
+
+    @Injectable({providedIn: 'root'})
+    class FooServiceDep {
+      name = 'FooServiceDep';
+    }
+
+    @Injectable({providedIn: 'root', useClass: BarService})
+    class FooService {
+      constructor(public dep: FooServiceDep) {}
+      getMessage() { return 'foo'; }
+    }
+
+    it('should use @Injectable useClass config when token is not provided', () => {
+      let provider: FooService|BarService;
+
+      @Component({template: ''})
+      class App {
+        constructor(service: FooService) { provider = service; }
+      }
+
+      TestBed.configureTestingModule({declarations: [App]});
+      const fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
+
+      expect(provider !.getMessage()).toBe('bar');
+
+      // ViewEngine incorrectly uses the original class DI config, instead of the one from useClass.
+      if (ivyEnabled) {
+        expect(provider !.dep.name).toBe('BarServiceDep');
+      }
+    });
+
+    it('should use constructor config directly when token is explicitly provided via useClass',
+       () => {
+         let provider: FooService|BarService;
+
+         @Component({template: ''})
+         class App {
+           constructor(service: FooService) { provider = service; }
+         }
+
+         TestBed.configureTestingModule(
+             {declarations: [App], providers: [{provide: FooService, useClass: FooService}]});
+         const fixture = TestBed.createComponent(App);
+         fixture.detectChanges();
+
+         expect(provider !.getMessage()).toBe('foo');
+       });
+
+
+    it('should inject correct provider when re-providing an injectable that has useClass', () => {
+      let directProvider: FooService|BarService;
+      let overriddenProvider: FooService|BarService;
+
+      @Component({template: ''})
+      class App {
+        constructor(@Inject('stringToken') overriddenService: FooService, service: FooService) {
+          overriddenProvider = overriddenService;
+          directProvider = service;
+        }
+      }
+
+      TestBed.configureTestingModule(
+          {declarations: [App], providers: [{provide: 'stringToken', useClass: FooService}]});
+      const fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
+
+      expect(directProvider !.getMessage()).toBe('bar');
+      expect(overriddenProvider !.getMessage()).toBe('foo');
+
+      // ViewEngine incorrectly uses the original class DI config, instead of the one from useClass.
+      if (ivyEnabled) {
+        expect(directProvider !.dep.name).toBe('BarServiceDep');
+        expect(overriddenProvider !.dep.name).toBe('FooServiceDep');
+      }
+    });
+
+    it('should use constructor config directly when token is explicitly provided as a type provider',
+       () => {
+         let provider: FooService|BarService;
+
+         @Component({template: ''})
+         class App {
+           constructor(service: FooService) { provider = service; }
+         }
+
+         TestBed.configureTestingModule({declarations: [App], providers: [FooService]});
+         const fixture = TestBed.createComponent(App);
+         fixture.detectChanges();
+
+         expect(provider !.getMessage()).toBe('foo');
+         expect(provider !.dep.name).toBe('FooServiceDep');
+       });
+  });
+
   describe('inject', () => {
 
     it('should inject from parent view', () => {

packages/core/test/acceptance/providers_spec.ts

@@ -467,29 +467,29 @@ describe('providers', () => {
     });
 
 
-    onlyInIvy('VE bug (see FW-1454)')
-        .it('should support forward refs in useClass when token is provided', () => {
-
-          @Injectable({providedIn: 'root', useClass: forwardRef(() => SomeProviderImpl)})
-          abstract class SomeProvider {
-          }
+    it('should support forward refs in useClass when token is provided', () => {
+      @Injectable({providedIn: 'root'})
+      abstract class SomeProvider {
+      }
 
-          @Injectable()
-          class SomeProviderImpl extends SomeProvider {
-          }
+      @Injectable()
+      class SomeProviderImpl extends SomeProvider {
+      }
 
-          @Component({selector: 'my-app', template: ''})
-          class App {
-            constructor(public foo: SomeProvider) {}
-          }
+      @Component({selector: 'my-app', template: ''})
+      class App {
+        constructor(public foo: SomeProvider) {}
+      }
 
-          TestBed.configureTestingModule(
-              {declarations: [App], providers: [{provide: SomeProvider, useClass: SomeProvider}]});
-          const fixture = TestBed.createComponent(App);
-          fixture.detectChanges();
+      TestBed.configureTestingModule({
+        declarations: [App],
+        providers: [{provide: SomeProvider, useClass: forwardRef(() => SomeProviderImpl)}]
+      });
+      const fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
 
-          expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl);
-        });
+      expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl);
+    });
 
   });