Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False "Missing required @injectable annotation in..." exception #247

Closed
remojansen opened this issue Jun 15, 2016 · 1 comment
Closed

False "Missing required @injectable annotation in..." exception #247

remojansen opened this issue Jun 15, 2016 · 1 comment
Assignees
Labels
Milestone

Comments

@remojansen
Copy link
Member

Expected Behavior

The following should work or throw an error that helps developers:

@injectable()
class Samurai implements ISamurai {

    public rank: string;

    public constructor(rank: string) {
        this.rank = rank;
    }
}

@injectable()
class SamuraiMaster extends Samurai implements ISamurai {
    constructor() {
        super("Master");
    }
}

Current Behavior

The current error is misleading:

Missing required @Injectable annotation in: SamuraiMaster.

Possible Solution

I don't think we are going to be able to implement this is an efficient/elegant manner so the best thing we can do is to provide a more friendly error. Something like:

When extending a class both parent and child classes must have the same number of constructor arguments. Read https://github.com/inversify/InversifyJS/blob/master/wiki/inheritance.md for more info.

@remojansen
Copy link
Member Author

remojansen commented Jun 20, 2016

I've been thinking about this over the weekend and the best solution that I have found is to restrict the number of constructor arguments in a derived class to be > than the number of constructor arguments of its base class.

We have the following cases (being length the number of constructor arguments):

  • A) Base.length > Derived.length (potential non-detectable error)
  • B) Base.length === Derived.length (no potential errors)
  • C) Base.length < Derived.length (no potential errors)

This issue is related with case A:

@injectable()
class Warrior {
    public rank: string;
    public constructor(rank: string) { // length = 1
        this.rank = rank;
    }
}

@injectable()
class SamuraiMaster extends Warrior  {
    public constructor() { // length = 0
       super("master");
    }
}

The code snippet above throw a misleading error:

Error: Derived class must explicitly declare its constructor: SamuraiMaster

We try to provide developers with useful feedback like:

Missing required @Injectable annotation in: SamuraiMaster

This works fine in most cases but it causes some problem when using inheritance. In order to overcome this issues we need to restrict the usage of inheritance.

The number of constructor arguments in a derived class MUST be >= than the number of constructor arguments of its base class.

The users have a few ways to overcome this limitation available. These will be documented in the wiki:

WORKAROUND A) Property setter

You can use the public, protected or private access modifier and a property setter:

@injectable()
class Warrior {
    protected rank: string;
    public constructor() { // length = 0
        this.rank = null;
    }
}

@injectable()
class SamuraiMaster extends Warrior {
    public constructor() { // length = 0
       super();
       this.rank = "master";
    }
}

WORKAROUND B) Property injection

@injectable()
class Warrior {
    protected rank: string;
    public constructor() { // length = 0
        this.rank = null;
    }
}

let TYPES = { Rank: "Rank" };

@injectable()
class SamuraiMaster extends Warrior  {

    @injectNamed(TYPES.Rank, "master")
    @named("master")
    protected rank: string;

    public constructor() { // length = 0
       super();
    }
}

kernel.bind<string>(TYPES.Rank)
      .toConstantValue("master")
      .whenTargetNamed("master");

WORKAROUND C) Inject into the derived class

@injectable()
class Warrior {
    protected rank: string;
    public constructor(rank: string) { // length = 1
        this.rank = rank;
    }
}

let TYPES = { Rank: "Rank" };

@injectable()
class SamuraiMaster extends Warrior  {
    public constructor(
        @inject(TYPES.Rank) @named("master") rank: string
    ) { // length = 1
       super(rank);
    }
}

kernel.bind<string>(TYPES.Rank)
      .toConstantValue("master")
      .whenTargetNamed("master");

The following should also work:

@injectable()
class Warrior {
    protected rank: string;
    public constructor(rank: string) { // length = 1
        this.rank = rank;
    }
}

interface Weapon {
    name: string;
}

@injectable()
class Katana implements Weapon {
    public name: string;
    public constructor() {
        this.name = "Katana";
    }
}

let TYPES = { 
    Rank: "Rank",
    Weapon: "Weapon"
};

@injectable()
class SamuraiMaster extends Warrior  {
    public weapon: Weapon;
    public constructor(
        @inject(TYPES.Rank) @named("master") rank: string,
        @inject(TYPES.Weapon) weapon: Weapon
    ) { // length = 2
       super(rank);
       this.weapon = weapon;
    }
}

kernel.bind<Weapon>(TYPES.Weapon).to(Katana);

kernel.bind<string>(TYPES.Rank)
      .toConstantValue("master")
      .whenTargetNamed("master");

So i'm going to implement the restriction and document this on the wiki. I'm hoping to get this issue resolved this week.

@remojansen remojansen mentioned this issue Jun 20, 2016
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant