inversify / InversifyJS

A powerful and lightweight inversion of control container for JavaScript & Node.js apps powered by TypeScript.
http://inversify.io/
MIT License
11.34k stars 719 forks source link

Property Multi-Injection Not Working ? #342

Closed anthonyjlmorel closed 8 years ago

anthonyjlmorel commented 8 years ago

I tried to make up a demo for using the lib and while I tried the property multi injection, it does not seems to work.

Expected Behavior

I have a class that has been injected with dependencies through property multi injection (instead of constructor multi injection which is working perfectly fine).

I expect inversify to resolve the dependencies just like it does with constructor injection.

Current Behavior

In my example, the array of dependencies that should be returned by inversify is undefined when I execute the code that needs it.

Steps to Reproduce (for bugs)

I posted my classes here, but as I can't understand why the highlighting is not working, I put a link toward a working example on my github, but you need to put the decorator out of the constructor :

// vehicle.ts
import {injectable, inject, multiInject} from "inversify";

@injectable()
export class Vehicle implements IVehicle{
    // Here, this does not work
    @multiInject("wheels") private wheels: IWheel[];

       // the tag was here before, and working
    constructor(@inject("engine") private engine: IEngine){
    }

    public startEngine(): void{
        this.engine.start();
    }

    public checkWheels(): void{
        if(Array.isArray(this.wheels)){
            this.wheels.forEach((wheel: IWheel, index: number) => {
                console.log("Checking wheel " + index);
                wheel.isTireFlat();
            });
        }
    }
}
// engine.ts
import {injectable, inject} from "inversify";

@injectable()
export class Engine implements IEngine {

    public start(): void{
        console.log("Starting Engine");
    }

    public stop(): void{
        // ...
    }

}
// wheel.ts
import {injectable } from 'inversify';

// Annotation mandatory for base class
@injectable()
class Wheel implements IWheel {

    public isTireFlat(): boolean{
        console.log("Checking if I am flat ...");
        return false;
    }

}

@injectable()
export class RearLeftWheel extends Wheel{
    // ...
}

@injectable()
export class RearRightWheel extends Wheel{
    // ...
}

@injectable()
export class FrontLeftWheel extends Wheel{
    // ...
}

@injectable()
export class FrontRightWheel extends Wheel{
    // ...
}
// dependency-loader.ts
import { Kernel, interfaces } from 'inversify';

import {Engine} from './engine';
import {Vehicle} from './vehicle';
import {RearLeftWheel, RearRightWheel, FrontLeftWheel, FrontRightWheel} from './wheel';

var kernel = new Kernel();

kernel.bind<IVehicle>("vehicle").to(Vehicle);
kernel.bind<IEngine>("engine").to(Engine);

[RearLeftWheel, RearRightWheel, FrontLeftWheel, FrontRightWheel].forEach((constr: interfaces.Newable<IWheel>)=>{
    kernel.bind<IWheel>("wheels").to(constr);
});

export default kernel;
// client.ts
import kernel from './dependencies-loader';
import "reflect-metadata";

var car: IVehicle = kernel.get<IVehicle>("vehicle");

car.checkWheels();
car.startEngine();

And the interfaces :

// interfaces.d.ts
interface IWheel{

    isTireFlat(): boolean;

}

interface IVehicle{
    startEngine(): void;
    checkWheels(): void;
}

interface IEngine{
    start(): void;
    stop(): void;
}

Context

I am trying out the framework to learn how to use it.

Your Environment

remojansen commented 8 years ago

Hi thanks for reporting I'm testing now I will let you know soon If I find out what it the problem. To highlight code you need to use:

screen shot 2016-08-16 at 21 43 59

remojansen commented 8 years ago

Hi, I think I know what is the problem. The decorator @multiInject() is a parameter decorator not a property decorator. It can be used to decorate the arguments of a constructor but not the properties of a class.

We had support for @inject() and @multiInject() but we recently removed it to a separated repository.

If you apply @multiInject() to a property:

 @multiInject("wheels") private wheels: IWheel[];

You will get a compilation error but this is the expected behaviour.

This week we have started to work on a new implementation of property injection. So for the moment you can only use:

But in a few weeks you will be able to use:

You can learn more details at the wiki.

anthonyjlmorel commented 8 years ago

Thanks @remojansen ! Can I ask for a clarification on lazy injection meaning ?

I was wondering if lazy injection can be a mean to resolve circular dependency (CD) error that inversify currently throws. In my sense, I see that CD can be a problem while in the constructor we want to directly use the dependency that has not been loaded yet and thus creates a kind of deadlock. But if I use my dependency later in my code (in a business method for instance), I think this CD is not an issue. Can I solve this by using the "lazy injection" you're talking about ?

Thanks for this amazing framework by the way.

remojansen commented 8 years ago

Thanks!

We have two options:

A) Using inversify property injection (Work in progress)

class Vehicle implements IVehicle{
    @multiInject("wheels") public wheels: IWheel[];
    constructor(@inject("engine") public engine: IEngine){}
}
let car= kernel.get<Vehicle>("Vehicle");
// At this point both the engine and the wheels have already been injected

B) Using inversify-inject-decorators lazy property injection

class Vehicle implements IVehicle{
    @lazyMultiInject("wheels") public wheels: IWheel[];
    constructor(@lazyInject("engine") public engine: IEngine){}
}
let car= kernel.get<Vehicle>("Vehicle");
// At this point only engine has already been injected
// When we try to access the property is when it is injected
console.log(car.wheels); // Wheels is injected when this line is executed

Keep in mind that when working with OOP in JavaScript you can have two kinds of circular dependencies:

If you have circular dependencies in your JavaScript modules weird things will happen (See an example). We can try to help with this kind of cases but there is not much we can do because it is purely based on your project organization. Some module loaders include features to fix circular dependencies in JS modules this is not the job of InversifyJS.

If you have a circular dependency in your JavaScript classes, InversifyJS will detect it and throw an exception so you can fix it.

Feel free to ask any other questions :smile:

jacks50 commented 7 years ago

Is the support for the @multiInject decorator resolved ? I've got a weird issue and it seems to concern it after trying to debug my app. I can't @multiInject properties nor constructor parameters, I've posted on SO to see if someone has had this issue or not ...

EDIT : It seems related to it because when deleting the decorator, my app runs without errors, else I got an error without message (as seen in the debugger)

remojansen commented 7 years ago

Hi @Jacks50 I don't see @multiInject in the code in your SO question? If you are binding 2 items to one identifier:

container.bind<IX>("IX").to(XA);
container.bind<IX>("IX").to(XB);

The get method as container.get<IX>("IX") will throw an error. If you want to resolve all you will need to use container.getAll<IX>("IX") instead.

jacks50 commented 7 years ago

Hi @remojansen and thank you for your quick reply.

Indeed, I forgot a class in my SO question :) I updated it, but after digging the issue I think now it is related to one of my injectable KeyFeature.

In fact, when I remove all the @inject in my KeyFeature classes (related to the SO question), I've got no more errors, but I made a small test to check if the @multiInject was working correctly and it was the case.

So yes, I am binding 2 items to one identifier, but they should be injected in the constructor with @multiInject. They are injected as long as none of the injected object has an @inject on its properties / constructor.

Does the little stacktrace gives you an hint about what could be the error ? I don't even have a message, is there a way to get one ? Or is this error not handled, and thus not giving a message ?

Thank you very much for your time !

jacks50 commented 7 years ago

I have finally found my error ! In my workflow, I'm binding some values retrieved from a JSON object which are typeof any and, as I guess, are not supported by inversify (since they are not newable).

Is it possible to bind primitive types (or any) so I can inject these values to the required objects ? Or do I must create a class for these values ?

remojansen commented 7 years ago

@Jacks50 you could bind it as a constant value:

container.bind<IX>("IX").toConstantValue(someVal);
jacks50 commented 7 years ago

@remojansen Thank you ! I just noticed it and used it, works perfectly ! Thank you very much for your time :)