Skip to content
Permalink
Browse files

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 angular/ngcc-validation#526

PR Close #33732
  • Loading branch information
petebacondarwin authored and kara committed Nov 11, 2019
1 parent b75a212 commit 4ec079f8528c8b44d89fe752e2d4acc6682c3242
@@ -160,6 +160,32 @@ describe('ngInjectableDef Bazel Integration', () => {
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', () => {
TestBed.configureTestingModule({
providers: [{provide: 'foo', useValue: 'bar'}],
@@ -190,14 +190,22 @@ export function ɵɵdefineInjector(options: {factory: () => any, providers?: any
* @param type A type which may have its own (non-inherited) `ɵprov`.
*/
export function getInjectableDef<T>(type: any): ɵɵInjectableDef<T>|null {
const def = (type[NG_PROV_DEF] || type[NG_INJECTABLE_DEF]) as ɵɵInjectableDef<T>;
// The definition read above may come from a base class. `hasOwnProperty` is not sufficient to
// 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
// 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.
return getOwnDefinition(type, type[NG_PROV_DEF]) ||
getOwnDefinition(type, type[NG_INJECTABLE_DEF]);
}

/**
* Return `def` only if it is defined directly on `type` and is not inherited from a base
* class of `type`.
*
* 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;
}

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

0 comments on commit 4ec079f

Please sign in to comment.
You can’t perform that action at this time.