From 40a0666651331d427069cc770a15a875822e6db4 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 18 May 2019 10:10:19 +0200 Subject: [PATCH] fix(ivy): error when using forwardRef in Injectable's useClass (#30532) Fixes Ivy throwing an error when something is passed in as a `forwardRef` into `@Injectable`'s `useClass` option. The error was being thrown, because we were trying to get the provider factory off of the wrapper function, rather than the value itself. This PR resolves FW-1335. PR Close #30532 --- packages/core/src/di/forward_ref.ts | 14 +++--- packages/core/src/di/jit/environment.ts | 9 ++++ packages/core/src/render3/di.ts | 9 ++++ .../core/test/acceptance/providers_spec.ts | 49 +++++++++++++++++++ .../injection/bundle.golden_symbols.json | 3 ++ .../bundling/todo/bundle.golden_symbols.json | 3 ++ 6 files changed, 80 insertions(+), 7 deletions(-) diff --git a/packages/core/src/di/forward_ref.ts b/packages/core/src/di/forward_ref.ts index d1de311b54..65708c7a4d 100644 --- a/packages/core/src/di/forward_ref.ts +++ b/packages/core/src/di/forward_ref.ts @@ -57,11 +57,11 @@ export function forwardRef(forwardRefFn: ForwardRefFn): Type { * @publicApi */ export function resolveForwardRef(type: T): T { - const fn: any = type; - if (typeof fn === 'function' && fn.hasOwnProperty(__forward_ref__) && - fn.__forward_ref__ === forwardRef) { - return fn(); - } else { - return type; - } + return isForwardRef(type) ? type() : type; +} + +/** Checks whether a function is wrapped by a `forwardRef`. */ +export function isForwardRef(fn: any): fn is() => any { + return typeof fn === 'function' && fn.hasOwnProperty(__forward_ref__) && + fn.__forward_ref__ === forwardRef; } diff --git a/packages/core/src/di/jit/environment.ts b/packages/core/src/di/jit/environment.ts index 302a2eec70..3d2fc5a497 100644 --- a/packages/core/src/di/jit/environment.ts +++ b/packages/core/src/di/jit/environment.ts @@ -7,6 +7,7 @@ */ import {Type} from '../../interface/type'; +import {isForwardRef, resolveForwardRef} from '../forward_ref'; import {ɵɵinject} from '../injector_compatibility'; import {getInjectableDef, getInjectorDef, ɵɵdefineInjectable, ɵɵdefineInjector} from '../interface/defs'; @@ -26,6 +27,14 @@ export const angularCoreDiEnv: {[name: string]: Function} = { function getFactoryOf(type: Type): ((type?: Type) => T)|null { const typeAny = type as any; + + if (isForwardRef(type)) { + return (() => { + const factory = getFactoryOf(resolveForwardRef(typeAny)); + return factory ? factory() : null; + }) as any; + } + const def = getInjectableDef(typeAny) || getInjectorDef(typeAny); if (!def || def.factory === undefined) { return null; diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index d481ef1290..ec754f7dd0 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {isForwardRef, resolveForwardRef} from '../di/forward_ref'; import {InjectionToken} from '../di/injection_token'; import {Injector} from '../di/injector'; import {injectRootLimpMode, setInjectImplementation} from '../di/injector_compatibility'; @@ -633,6 +634,14 @@ export class NodeInjector implements Injector { */ export function ɵɵgetFactoryOf(type: Type): FactoryFn|null { const typeAny = type as any; + + if (isForwardRef(type)) { + return (() => { + const factory = ɵɵgetFactoryOf(resolveForwardRef(typeAny)); + return factory ? factory() : null; + }) as any; + } + const def = getComponentDef(typeAny) || getDirectiveDef(typeAny) || getPipeDef(typeAny) || getInjectableDef(typeAny) || getInjectorDef(typeAny); if (!def || def.factory === undefined) { diff --git a/packages/core/test/acceptance/providers_spec.ts b/packages/core/test/acceptance/providers_spec.ts index 4963b87e7e..581b2080f4 100644 --- a/packages/core/test/acceptance/providers_spec.ts +++ b/packages/core/test/acceptance/providers_spec.ts @@ -9,6 +9,7 @@ import {Component, Directive, Inject, Injectable, InjectionToken, Injector, NgModule, Optional, forwardRef} from '@angular/core'; import {TestBed, async, inject} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; +import {expect} from '@angular/platform-browser/testing/src/matchers'; import {onlyInIvy} from '@angular/private/testing'; describe('providers', () => { @@ -319,6 +320,54 @@ describe('providers', () => { expect(fixture.componentInstance.myService.dep.value).toBe('one'); }); + it('should support forward refs in useClass when impl version is also provided', () => { + + @Injectable({providedIn: 'root', useClass: forwardRef(() => SomeProviderImpl)}) + abstract class SomeProvider { + } + + @Injectable() + class SomeProviderImpl extends SomeProvider { + } + + @Component({selector: 'my-app', template: ''}) + class App { + constructor(public foo: SomeProvider) {} + } + + TestBed.configureTestingModule( + {declarations: [App], providers: [{provide: SomeProvider, useClass: SomeProviderImpl}]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl); + }); + + + 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 { + } + + @Injectable() + class SomeProviderImpl extends 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(); + + expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl); + }); + }); describe('flags', () => { diff --git a/packages/core/test/bundling/injection/bundle.golden_symbols.json b/packages/core/test/bundling/injection/bundle.golden_symbols.json index ac75847f48..55b71159e8 100644 --- a/packages/core/test/bundling/injection/bundle.golden_symbols.json +++ b/packages/core/test/bundling/injection/bundle.golden_symbols.json @@ -155,6 +155,9 @@ { "name": "isFactoryProvider" }, + { + "name": "isForwardRef" + }, { "name": "isTypeProvider" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index da6a18a1df..8bb5afa130 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1043,6 +1043,9 @@ { "name": "isFactory" }, + { + "name": "isForwardRef" + }, { "name": "isJsObject" },