Inversifyjs: Injectable class that extends a base class with an unmanaged constructor but requires an injected parameter in its own constructor does not get injected parameter without explicitly using @inject

[IRCraziestTaxi]

Sorry for the long winded title. Here is the gist of what is going on. I recently had an issue resolved in which I am doing the following:

@injectable()
export class RepoBase<T> implements IRepoBase {
  protected readonly _repository: Repository<T>;

  constructor(@unmanaged() entityType: { new (): T; }) {
    this._repository = getConnectionManager().get().getRepository<T>(entityType);
  }
}

@injectable()
export class TypedRepo extends RepoBase<Type> implements ITypedRepo {
  constructor() {
    super(Type);
  }
}

I tried to follow suit by doing the following:

@injectable()
export class BLBase<T> implements IBLBase {
  protected _repository: IRepoBase<T>;

  constructor(@unmanaged() repository: IRepoBase<T>) {
    this._repository = repository;
  }
}

@injectable()
export class TypedBL extends BLBase<Type> implements ITypedBL {
  constructor(repository: TypedRepo) { // Not injected
    super(repository);
  }
}

However, TypedRepo is not injected into TypedBL. I must explicitly use @inject in TypedBL's constructor:

@injectable()
export class TypedBL extends BLBase<Type> implements ITypedBL {
  constructor(@inject(TypedRepo) repository: TypedRepo) { // Injected
    super(repository);
  }
}

Then it is injected. Has this been brought up before? I don't feel right having to explicitly use @inject in some places but not in most.

Edit:

I forgot to add an interesting caveat: if TypedBL no longer extends BLBase, then TypedRepo is injected without the need for @inject:

@injectable()
export class TypedBL implements ITypedBL { // Does not extend BLBase
  constructor(repository: TypedRepo) { // Injected
    super(repository);
  }
}

[remojansen]

Hi, I have one question. It is TypedRepo a class or an interface? Inversify requires @inject annotations always if you use interfaces. Annotations are not required if you use classes instead of interfaces. This is due to the way the TypeScript compiler generates metadata.

[IRCraziestTaxi]

@remojansen TypedRepo is a class. I forgot to mention that if I make TypedBL a standalone class instead of extending BLBase, then TypedRepo is injected without the need for @inject. It is when TypedBL extends BLBase that @inject becomes necessary.

[remojansen]

I will debug a test case to see if we can make it optional but it is likely to not be possible. We had issue with silent errors when using inheritance and we added some rules to ensure that there will never be silent errors. The problems is that those rules make the way inheritance work more restrictive and this seems to be one of those limitations.

[remojansen]

So I managed to get some free time to test this during the weekend and I found the problem:

@injectable()
export class BLBase<T> implements IBLBase {
  protected _repository: IRepoBase<T>;

  constructor(repository: IRepoBase<T>) { // REMOVED @unmanaged()  !!!
    this._repository = repository;
  }
}

Then you can use extends BLBase<Type> without problems:

@injectable()
class TypedBL extends BLBase<Type> implements ITypedBL {
  constructor(@inject(TypedRepo) repository: TypedRepo) {
    super(repository);
  }
}

I have added a test case with some comments here. It should help you to understand better when to use @unmanaged.

[ggranum]

@remojansen: Reading the docs, I get the impression there is an assumption (in the code) that a base-class will be used as a bind target, even if/when it never actually is. Am I wrong?

Angular doesn't have this restriction (forced declaration of inject on the child class ctor args), indicating it's possible, so obviously they are doing something different. Presumably related to the fact that abstract base classes do not require an @Inject annotation?

This issue is quite confusing and unexpected, in great part because everything else (at least that I've worked with so far) Just Works, which is, of course, great :~).

Thanks much,

[garkin]

Why am i forced to annotate base class if it's just a code reuse and not injected anywhere?

[remojansen]

@ggranum sorry for the very late reply, I think I missed the comment notification somehow... I just saw this because @garkin added a new comment.

The annotations are required because we have many use cases and kinds of mistakes:

• Base class don't have any dependencies
• Derived class don't have any dependencies
• Both Base and Derived classes have dependencies
• Sometimes users forget the injectable annotation in the Base class.
• Sometimes users forget the injectable annotation in the Derived class.
• Sometimes users forget an inject annotation in the Base class.
  
  
  
  • Sometimes users forget an inject annotation in the Derived class.
  
  

I tried to create errors to let the users know what was exactly the problem but it was not possible for me to identify the exact problem if only the Derived class was annotated. So we decided that forcing some restrictions in order to achieve better error reporting was a good idea.

[garkin]

Would be cool to see an option to lift off this assertion:

new Container({ assertInheritanceInjection: false })

It realy bogs me to write boilerplate annotations and polute dependency space to make framework feel good about already idiomaticaly correct code.
Great experience with Inversify so far, thank you for your job.

[garkin]

If someone is still wondering what exactly is wrong with OPs code.

The problem is that default MetadataReader uses Reflect.getMetadata instead of Reflect.getOwnMetadata.

Reflect.getMetadata does search for own metadata of the class, and if it found none it will follow inheritance tree upwards untill it will find one.
So, when Inversify tries to retrieve metadata of TypedBL with no annotations in constructor it receives BLBase metadata instead with @unmanaged() on the first constructor argument.

https://github.com/rbuckton/reflect-metadata/blob/master/Reflect.ts#L1263

 function OrdinaryGetMetadata(MetadataKey: any, O: any, P: string | symbol | undefined): any {
    const hasOwn = OrdinaryHasOwnMetadata(MetadataKey, O, P);
    if (hasOwn) return OrdinaryGetOwnMetadata(MetadataKey, O, P);
    const parent = OrdinaryGetPrototypeOf(O);
    if (!IsNull(parent)) return OrdinaryGetMetadata(MetadataKey, parent, P);
    return undefined;
}

As already was mentioned before, there are following ways to avoid this:

• provide own implementation of MetadataReader (just ugly)
• remove @unmanaged() from the base class (will limit possible minimum constructor arguments count for children, could be not applicable for more complex inheritance topologies)
• add at least one explicit @inject(...) for the children constructor, to make Reflect.getMetadata happy (that way we lose declared ability to infer dependencies from constructor argument types )