fix(ivy): support sub-class services that only inherit `@Injectable` (#30388)

In View engine it is possible to instantiate a service that that has no
`@Injectable` decorator as long as it satisfies one of:

1) It has no dependencies and so a constructor with no parameters.
This is already supported in Ivy.
2) It has no constructor of its own and sub-classes a service which has
dependencies but has its own `@Injectable` decorator. This second
scenario was broken in Ivy.

In Ivy, previous to this commit, if a class to be instantiated did not have
its own `@Injectable` decorator and did not provide a constructor of
its own, then it would be created using `new` with no arguments -
i.e. falling back to the first scenario.

After this commit Ivy correctly uses the `ngInjectableDef` inherited
from the super-class to provide the `factory` for instantiating the
sub-class.

FW-1314

PR Close #30388
This commit is contained in:
Pete Bacon Darwin 2019-05-13 19:43:11 +01:00 committed by Jason Aden
parent 35c1750fcc
commit 1a0e500eea
4 changed files with 113 additions and 24 deletions

View File

@ -182,12 +182,34 @@ export function ΔdefineInjector(options: {factory: () => any, providers?: any[]
}
/**
* Read the `ngInjectableDef` type in a way which is immune to accidentally reading inherited value.
* Read the `ngInjectableDef` for `type` in a way which is immune to accidentally reading inherited
* value.
*
* @param type type which may have `ngInjectableDef`
* @param type A type which may have its own (non-inherited) `ngInjectableDef`.
*/
export function getInjectableDef<T>(type: any): ΔInjectableDef<T>|null {
return type && type.hasOwnProperty(NG_INJECTABLE_DEF) ? (type as any)[NG_INJECTABLE_DEF] : null;
return type && type.hasOwnProperty(NG_INJECTABLE_DEF) ? type[NG_INJECTABLE_DEF] : null;
}
/**
* Read the `ngInjectableDef` for `type` or read the `ngInjectableDef` from one of its ancestors.
*
* @param type A type which may have `ngInjectableDef`, via inheritance.
*
* @deprecated Will be removed in v10, where an error will occur in the scenario if we find the
* `ngInjectableDef` on an ancestor only.
*/
export function getInheritedInjectableDef<T>(type: any): ΔInjectableDef<T>|null {
if (type && type[NG_INJECTABLE_DEF]) {
// TODO(FW-1307): Re-add ngDevMode when closure can handle it
// ngDevMode &&
console.warn(
`DEPRECATED: DI is instantiating a token "${type.name}" that inherits its @Injectable decorator but does not provide one itself.\n` +
`This will become an error in v10. Please add @Injectable() to the "${type.name}" class.`);
return type[NG_INJECTABLE_DEF];
} else {
return null;
}
}
/**

View File

@ -15,7 +15,7 @@ import {resolveForwardRef} from './forward_ref';
import {InjectionToken} from './injection_token';
import {Injector} from './injector';
import {INJECTOR, NG_TEMP_TOKEN_PATH, NullInjector, THROW_IF_NOT_FOUND, USE_VALUE, catchInjectorError, injectArgs, setCurrentInjector, Δinject} from './injector_compatibility';
import {InjectableType, InjectorType, InjectorTypeWithProviders, getInjectableDef, getInjectorDef, ΔInjectableDef} from './interface/defs';
import {InjectorType, InjectorTypeWithProviders, getInheritedInjectableDef, getInjectableDef, getInjectorDef, ΔInjectableDef} from './interface/defs';
import {InjectFlags} from './interface/injector';
import {ClassProvider, ConstructorProvider, ExistingProvider, FactoryProvider, StaticClassProvider, StaticProvider, TypeProvider, ValueProvider} from './interface/provider';
import {APP_ROOT} from './scope';
@ -378,25 +378,52 @@ export class R3Injector {
}
function injectableDefOrInjectorDefFactory(token: Type<any>| InjectionToken<any>): () => any {
const injectableDef = getInjectableDef(token as InjectableType<any>);
if (injectableDef === null) {
const injectorDef = getInjectorDef(token as InjectorType<any>);
if (injectorDef !== null) {
return injectorDef.factory;
} else if (token instanceof InjectionToken) {
throw new Error(`Token ${stringify(token)} is missing an ngInjectableDef definition.`);
} else if (token instanceof Function) {
const paramLength = token.length;
if (paramLength > 0) {
const args: string[] = new Array(paramLength).fill('?');
throw new Error(
`Can't resolve all parameters for ${stringify(token)}: (${args.join(', ')}).`);
}
return () => new (token as Type<any>)();
}
throw new Error('unreachable');
// Most tokens will have an ngInjectableDef directly on them, which specifies a factory directly.
const injectableDef = getInjectableDef(token);
if (injectableDef !== null) {
return injectableDef.factory;
}
// If the token is an NgModule, it's also injectable but the factory is on its ngInjectorDef.
const injectorDef = getInjectorDef(token);
if (injectorDef !== null) {
return injectorDef.factory;
}
// InjectionTokens should have an ngInjectableDef and thus should be handled above.
// If it's missing that, it's an error.
if (token instanceof InjectionToken) {
throw new Error(`Token ${stringify(token)} is missing an ngInjectableDef definition.`);
}
// Undecorated types can sometimes be created if they have no constructor arguments.
if (token instanceof Function) {
return getUndecoratedInjectableFactory(token);
}
// There was no way to resolve a factory for this token.
throw new Error('unreachable');
}
function getUndecoratedInjectableFactory(token: Function) {
// If the token has parameters then it has dependencies that we cannot resolve implicitly.
const paramLength = token.length;
if (paramLength > 0) {
const args: string[] = new Array(paramLength).fill('?');
throw new Error(`Can't resolve all parameters for ${stringify(token)}: (${args.join(', ')}).`);
}
// The constructor function appears to have no parameters.
// This might be because it inherits from a super-class. In which case, use an ngInjectableDef
// from an ancestor if there is one.
// Otherwise this really is a simple class with no dependencies, so return a factory that
// just instantiates the zero-arg constructor.
const inheritedInjectableDef = getInheritedInjectableDef(token);
if (inheritedInjectableDef !== null) {
return inheritedInjectableDef.factory;
} else {
return () => new (token as Type<any>)();
}
return injectableDef.factory;
}
function providerToRecord(

View File

@ -10,7 +10,7 @@ import {CommonModule} from '@angular/common';
import {Attribute, ChangeDetectorRef, Component, Directive, ElementRef, EventEmitter, Host, HostBinding, INJECTOR, Inject, Injectable, Injector, Input, LOCALE_ID, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, forwardRef} from '@angular/core';
import {ViewRef} from '@angular/core/src/render3/view_ref';
import {TestBed} from '@angular/core/testing';
import {onlyInIvy} from '@angular/private/testing';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
describe('di', () => {
describe('no dependencies', () => {
@ -370,7 +370,7 @@ describe('di', () => {
<div dirA #dir="dirA">{{ dir.dirB.value }}</div>
</ng-template>
</div>
<div dirB value="insertion">
<div structuralDir [tmp]="foo"></div>
<!-- insertion point -->
@ -858,6 +858,40 @@ describe('di', () => {
const divElement = fixture.nativeElement.querySelector('div');
expect(divElement.textContent).toEqual('MyService');
});
it('should support sub-classes with no @Injectable decorator', () => {
@Injectable()
class Dependency {
}
@Injectable()
class SuperClass {
constructor(public dep: Dependency) {}
}
// Note, no @Injectable decorators for these two classes
class SubClass extends SuperClass {}
class SubSubClass extends SubClass {}
@Component({template: ''})
class MyComp {
constructor(public myService: SuperClass) {}
}
TestBed.configureTestingModule({
declarations: [MyComp],
providers: [{provide: SuperClass, useClass: SubSubClass}, Dependency]
});
const warnSpy = spyOn(console, 'warn');
const fixture = TestBed.createComponent(MyComp);
expect(fixture.componentInstance.myService.dep instanceof Dependency).toBe(true);
if (ivyEnabled) {
expect(warnSpy).toHaveBeenCalledWith(
`DEPRECATED: DI is instantiating a token "SubSubClass" that inherits its @Injectable decorator but does not provide one itself.\n` +
`This will become an error in v10. Please add @Injectable() to the "SubSubClass" class.`);
}
});
});
describe('inject', () => {

View File

@ -122,6 +122,9 @@
{
"name": "getGlobal"
},
{
"name": "getInheritedInjectableDef"
},
{
"name": "getInjectableDef"
},
@ -131,6 +134,9 @@
{
"name": "getNullInjector"
},
{
"name": "getUndecoratedInjectableFactory"
},
{
"name": "hasDeps"
},