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

Misleading error in circular dependency with @lazyInject #685

Closed remojansen closed 6 years ago

remojansen commented 7 years ago

The following circular dependency graph:

Test => Dom => DomUi => Dom (Lazy) => DomUi => Dom (Lazy) => ...
import "reflect-metadata";
import { Container, inject, injectable } from "inversify";
import getDecorators from "inversify-inject-decorators";

let container = new Container();
let { lazyInject } = getDecorators(container);

@injectable()
class Dom {
    public domUi: DomUi;
    constructor (domUi: DomUi) {
        this.domUi = domUi;
    }
}

@injectable()
class DomUi {
    @lazyInject(Dom) public dom: Dom;
}

@injectable()
class Test {
    constructor(dom: Dom) {
        console.log(dom);
    }
}

container.bind<Dom>(Dom).toSelf().inSingletonScope();
container.bind<DomUi>(DomUi).toSelf().inSingletonScope();
const dom = container.resolve(Test);

Throws:

Error: Missing required @inject or @multiInject annotation in: argument 0 in class Dom.

However, the following circular dependency graph:

Test => Dom => DomUi (Lazy) => Dom => DomUi (Lazy) => Dom => ...
import "reflect-metadata";
import { Container, inject, injectable } from "inversify";
import getDecorators from "inversify-inject-decorators";

let container = new Container();
let { lazyInject } = getDecorators(container);

@injectable()
class DomUi {
    public dom: Dom;
    constructor (dom: Dom) {
        this.dom = dom;
    }
}

@injectable()
class Dom {
    @lazyInject(DomUi)  public domUi: DomUi;
}

@injectable()
class Test {
    constructor(dom: Dom) {
        console.log(dom);
    }
}

container.bind<Dom>(Dom).toSelf().inSingletonScope();
container.bind<DomUi>(DomUi).toSelf().inSingletonScope();
const dom = container.resolve(Test);

Works:

Dom {}

Source: https://stackoverflow.com/questions/47415313/inversify-circular-singleton-injection/47416198

remojansen commented 6 years ago

Turns out that this is not an error.

This error may seem a bit misleading because when using classes as service identifiers @inject annotations should not be required and if we do add an annotation like @inject(Dom) or @inject(DomUi) we will still get the same exception. This happens because, at the point in time in which the decorator is invoked, the class has not been declared so the decorator is invoked as @inject(undefined). This trigger InversifyJS to think that the annotation was never added. The solution is to use Symbol.for("Dom") as service identifier instead of the class Dom.

I have added some test at https://github.com/inversify/inversify-inject-decorators/pull/40/

remojansen commented 6 years ago

I have documented this @ https://github.com/inversify/InversifyJS/blob/master/wiki/classes_as_id.md#known-limitation-classes-as-identifiers-and-circular-dependencies