inversify / InversifyJS

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

Support for @unmanaged() decorator #329

Closed remojansen closed 8 years ago

remojansen commented 8 years ago

Current Behavior

In some cases when working with derived classes we get an error:

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

This error is correct and was designed to prevent users from forgetting to inject all the required arguments into a Base class. However, in some cases the code is correct and no error should be thrown. The issue is caused because the container cannot find out how many arguments have been injected by the users (as opposed to be injected by the container).

Steps to Reproduce

const BaseId = "Base";

@injectable()
class Base {
    public prop: string;
    public constructor(arg: string) {
        this.prop = arg;
    }
}

@injectable()
class Derived extends Base {
    public constructor() {
        super("unmanaged-injected-value");
    }
}

let kernel = new Kernel();
kernel.bind<Base>(BaseId).to(Derived);
let tryGet = () => { kernel.get(BaseId); };
let error = ERROR_MSGS.ARGUMENTS_LENGTH_MISMATCH_1 + "Derived" + 
            ERROR_MSGS.ARGUMENTS_LENGTH_MISMATCH_2;
expect(tryGet).to.throw(error);

Expected Behavior

There should be a way to allow users to inject some of the arguments into the derived class and not get the error. So we have two kinds of arguments injected into a base class:

Because InversifyJS does not have control under user provided values and it doesn't manage their injection I though that a good name for a new annotation would be @unsafe() or @unmanaged(). For the moment I'm using @unmanaged() but I'm open to sugestions.

Possible Solution

We can use the @unmanaged() annotation to flag the arguments that will be not managed by the container, or in other words, the values that will be injected by the an user:

const BaseId = "Base";

@injectable()
class Base {
    public prop: string;
    public constructor(@unmanaged() arg: string) { // injected by user
        this.prop = arg;
    }
}

@injectable()
class Derived extends Base {
    public constructor() {
        super("unmanaged-injected-value"); // user injection
    }
}

let kernel = new Kernel();
kernel.bind<Base>(BaseId).to(Derived);
expect(kernel.get(BaseId) instanceof Base).to.eql(true);

We should support cases with multiple args:

const BaseId = "Base";

@injectable()
class Base {

    public prop1: string;
    public prop2: string;
    public prop3: string;

    public constructor(
        @unmanaged() arg1: string, // injected by user
        arg2: string, // injected by container
        arg3: string // injected by container
    ) {
        this.prop1 = arg1;
        this.prop2 = arg2;
        this.prop3 = arg3;
    }

}

@injectable()
class Derived extends Base {

    public constructor(
        @inject("something1") arg1: string, // container injection
        @inject("something2") arg2: string // container injection
    ) {
        super(
            "unmanaged-injected-value", // user injection
             arg1,
            arg2
        );
    }

}

let kernel = new Kernel();
kernel.bind<string>("something1").toConstantValue("test1");
kernel.bind<string>("something2").toConstantValue("test2");
kernel.bind<Base>(BaseId).to(Derived);
let instance = kernel.get(BaseId);
expect(instance  instanceof Base).to.eql(true);
expect(instance.prop1).to.eql("unmanaged-injected-value");

If you forget the @unmanaged() annotation an error will be thrown just like in the current implementation

Context

This issue has been reported many times so It is a high priority issue.

remojansen commented 8 years ago

Hi @otbe and @endel, since you asked me about this issue in the past I would appreciate your input on this. You can follow the progress here.

Thanks!

remojansen commented 8 years ago

I have implemented this at https://github.com/inversify/InversifyJS/pull/333