microsoft / tsyringe

Lightweight dependency injection container for JavaScript/TypeScript
MIT License
5.16k stars 173 forks source link

How to handle circular dependecy #38

Closed elerocks closed 4 years ago

elerocks commented 5 years ago

So let's say I have first class TestA that has dependency to class TestB

// TestA.ts
import { inject, injectable } from "tsyringe";
import { ITestB } from "./TestB";

export interface ITestA {
    helloA(): void;
    waveToB(): void;
}

@injectable()
export class TestA implements ITestA {
    constructor(@inject("ITestB") private testB: ITestB) { }
    /**
     * helloA
     */
    public helloA(): void {
        console.log("Hello from class A");
    }
    public waveToB(): void {
        console.log("waving to B");
        this.testB.helloB();
    }
}

and then I have second class TestB that has dependency to TestA:

// TestB.ts
import { inject, injectable } from "tsyringe";
import { ITestA } from "./TestA";

export interface ITestB {
    helloB(): void;
}

@injectable()
export class TestB implements ITestB {
    constructor(@inject("ITestA") private testA: ITestA) { }
    /**
     * helloB
     */
    public helloB(): void {
        console.log("Hello from class B");
    }
    public waveToA(): void {
        console.log("waving to A");
        this.testA.helloA();
    }
}

And finally main:

import { container } from "tsyringe";
import { TestA } from "./lib/TestA";
import { TestB } from "./lib/TestB";
const testA = container.resolve(TestA);
testA.helloA();
testA.waveToB();
const testB = container.resolve(TestB);
testB.helloB();

Which upon running results in "Maximum call stack size exceeded" and crashes. Is there any solution to this without restructuring the code? (sometimes it is not that simple). I've seen probable solution by @unlocomqx : https://github.com/microsoft/tsyringe/issues/20 but I'm unable to compile it due to errors.

unlocomqx commented 5 years ago

@elerocks check the lazy branch in my fork, I added the "lazyInject" decorator to solve the circular dep issue https://github.com/unlocomqx/tsyringe/tree/lazy You'll need to remove the injection from the constructor and write it as a property. Example: @lazyInject("DesignManager") protected designManager: DesignManager;

I didn't test it with interfaces though, but you could install with npm like this if you'd like to test it npm i git+https://github.com/unlocomqx/tsyringe.git#build

elerocks commented 5 years ago

you are lifesaver @unlocomqx, works like a charm. Only for classes though, not for interfaces (that'd be nice addition for next release :-) )

I think it'd be nice to merge into ms/syringe in some form or at least detect circular dependencies as it is very hard to find circular dependency in large project

unlocomqx commented 5 years ago

Check the new commits, you can install using the build2 branch npm i git+https://github.com/unlocomqx/tsyringe.git#build2

There is an example in the file /src/__tests__/lazy-inject.test.ts

export interface IChicken {
}

@injectable()
@registry([{
  token   : "IEgg",
  useToken: "EggWithInterface"
}])
export class ChickenWithInterface implements IChicken {

  @lazyInject("IEgg") public egg: IEgg;

  constructor() {
  }
}
export interface IEgg { }

@injectable()
@registry([{
  token: "IChicken",
  useToken: "ChickenWithInterface"
}])
export class EggWithInterface implements IEgg {

  @lazyInject("IChicken") public chicken: IChicken;

  constructor() {
  }
}

I will try to make a PR here but it's become a bit messy tbh :laughing:

GitStorageOne commented 5 years ago

@unlocomqx Are you still planning to make PR? Would be nice)

unlocomqx commented 5 years ago

@GitStorageOne I'm not satisified with the code tbh, if anyone wants to fork, refactor and make a PR, it would be fine by me :))