fix(core): support `ngInjectableDef` on types with inherited `ɵprov` (#33732)

The `ngInjectableDef` property was renamed to `ɵprov`, but core must
still support both because there are published libraries that use the
older term.

We are only interested in such properties that are defined directly on
the type being injected, not on base classes. So there is a check that
the defintion is specifically for the given type.

Previously if you tried to inject a class that had `ngInjectableDef` but
also inherited `ɵprov` then the check would fail on the `ɵprov` property
and never even try the `ngInjectableDef` property resulting in a failed
injection.

This commit fixes this by attempting to find each of the properties
independently.

Fixes https://github.com/angular/ngcc-validation/pull/526

PR Close #33732
This commit is contained in:
Pete Bacon Darwin 2019-11-11 15:01:58 +00:00 committed by Kara Erickson
parent 2f0b8bc541
commit 641c671bac
4 changed files with 48 additions and 8 deletions

View File

@ -160,6 +160,32 @@ describe('ngInjectableDef Bazel Integration', () => {
expect(() => TestBed.inject(ChildService).value).toThrowError(/ChildService/); expect(() => TestBed.inject(ChildService).value).toThrowError(/ChildService/);
}); });
it('uses legacy `ngInjectable` property even if it inherits from a class that has `ɵprov` property',
() => {
@Injectable({
providedIn: 'root',
useValue: new ParentService('parent'),
})
class ParentService {
constructor(public value: string) {}
}
// ChildServices exteds ParentService but does not have @Injectable
class ChildService extends ParentService {
constructor(value: string) { super(value); }
static ngInjectableDef = {
providedIn: 'root',
factory: () => new ChildService('child'),
token: ChildService,
};
}
TestBed.configureTestingModule({});
// We are asserting that system throws an error, rather than taking the inherited
// annotation.
expect(TestBed.inject(ChildService).value).toEqual('child');
});
it('NgModule injector understands requests for INJECTABLE', () => { it('NgModule injector understands requests for INJECTABLE', () => {
TestBed.configureTestingModule({ TestBed.configureTestingModule({
providers: [{provide: 'foo', useValue: 'bar'}], providers: [{provide: 'foo', useValue: 'bar'}],

View File

@ -190,14 +190,22 @@ export function ɵɵdefineInjector(options: {factory: () => any, providers?: any
* @param type A type which may have its own (non-inherited) `ɵprov`. * @param type A type which may have its own (non-inherited) `ɵprov`.
*/ */
export function getInjectableDef<T>(type: any): ɵɵInjectableDef<T>|null { export function getInjectableDef<T>(type: any): ɵɵInjectableDef<T>|null {
const def = (type[NG_PROV_DEF] || type[NG_INJECTABLE_DEF]) as ɵɵInjectableDef<T>; return getOwnDefinition(type, type[NG_PROV_DEF]) ||
// The definition read above may come from a base class. `hasOwnProperty` is not sufficient to getOwnDefinition(type, type[NG_INJECTABLE_DEF]);
// distinguish this case, as in older browsers (e.g. IE10) static property inheritance is }
// implemented by copying the properties.
// /**
// Instead, the ɵprov's token is compared to the type, and if they don't match then the * Return `def` only if it is defined directly on `type` and is not inherited from a base
// property was not defined directly on the type itself, and was likely inherited. The definition * class of `type`.
// is only returned if the type matches the def.token. *
* The function `Object.hasOwnProperty` is not sufficient to distinguish this case because in older
* browsers (e.g. IE10) static property inheritance is implemented by copying the properties.
*
* Instead, the definition's `token` is compared to the `type`, and if they don't match then the
* property was not defined directly on the type itself, and was likely inherited. The definition
* is only returned if the `type` matches the `def.token`.
*/
function getOwnDefinition<T>(type: any, def: ɵɵInjectableDef<T>): ɵɵInjectableDef<T>|null {
return def && def.token === type ? def : null; return def && def.token === type ? def : null;
} }

View File

@ -131,6 +131,9 @@
{ {
"name": "getNullInjector" "name": "getNullInjector"
}, },
{
"name": "getOwnDefinition"
},
{ {
"name": "getUndecoratedInjectableFactory" "name": "getUndecoratedInjectableFactory"
}, },

View File

@ -737,6 +737,9 @@
{ {
"name": "getOriginalError" "name": "getOriginalError"
}, },
{
"name": "getOwnDefinition"
},
{ {
"name": "getParentInjectorIndex" "name": "getParentInjectorIndex"
}, },