inversify / InversifyJS

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

Circular dependencies with LazyServiceIdentifier #1206

Open MurrayMeller opened 4 years ago

MurrayMeller commented 4 years ago

Hi all,

I''m not yet sure if this is a technical issue or a lack of understand on my part. Looking forward to learning from you all!

Expected Behaviour

https://github.com/inversify/InversifyJS/blob/master/wiki/circular_dependencies.md

What I understand from the linked article is that, Inversify can handle circular dependencies. That is, it should be able to handle a case where:

The article writes:

If you have a circular dependency between two modules and you use the @inject(SomeClass) annotation. At runtime, one module will be parsed before the other and the decorator could be invoked with @inject(SomeClass /* SomeClass = undefined*/). InversifyJS will throw an exception.

It then says there are two ways to overcome this limitation:

I have attempted a minimal code example of the LazyServiceIdentifier in this repository and have details below on how to produce the unexpected behaviour:

https://bitbucket.org/murraymeller/inversifycirculardependency/src/master/

Current Behaviour

ClubService.ts

...
@injectable()
export class ClubService implements IClubService {
  private userService;

  public constructor(
    @inject(new LazyServiceIdentifer(() => TYPES.UserService)) userService: IUserService,
  ) {
    this.userService = userService;
  }
}

UserService.ts

...
@injectable()
export class UserService implements IUserService {
  private clubService;

  public constructor(
    @inject(new LazyServiceIdentifer(() => TYPES.ClubService)) clubService: IClubService,
  ) {
    this.clubService = clubService;
  }
}

It will successfully compile but will crash when attempting to run. It gives an error (see stacktrace)

Am i correct in thinking that this should work?

Steps to Reproduce (for bugs)

See repo readme

Your Environment

Stack trace

/<PROJECT ROOT>/node_modules/inversify/lib/utils/serialization.js:65
            throw new Error(ERROR_MSGS.CIRCULAR_DEPENDENCY + " " + services);
            ^

Error: Circular dependency found: Symbol(ClubService) --> Symbol(UserService) --> Symbol(ClubService)
    at /<PROJECT ROOT>/node_modules/inversify/lib/utils/serialization.js:65:19
    at Array.forEach (<anonymous>)
    at circularDependencyToException (/<PROJECT ROOT>/node_modules/inversify/lib/utils/serialization.js:62:27)
    at /<PROJECT ROOT>/node_modules/inversify/lib/utils/serialization.js:68:13
    at Array.forEach (<anonymous>)
    at Object.circularDependencyToException (/<PROJECT ROOT>/node_modules/inversify/lib/utils/serialization.js:62:27)
    at Object.plan (/<PROJECT ROOT>/node_modules/inversify/lib/planning/planner.js:142:33)
    at /<PROJECT ROOT>/node_modules/inversify/lib/container/container.js:317:37
    at Container._get (/<PROJECT ROOT>/node_modules/inversify/lib/container/container.js:310:44)
    at Container.get (/<PROJECT ROOT>/node_modules/inversify/lib/container/container.js:230:21)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
sagitsofan commented 3 years ago

@MurrayMeller Did you overcome this issue?

MurrayMeller commented 3 years ago

@MurrayMeller Did you overcome this issue?

Hi @sagitsofan!

Sadly, I wasn't able to perform what I outlined in the question.

What I ended up doing was splitting up my two classes into several smaller classes. I was able to seperate them such that I no longer had circular dependencies.

Israfil22 commented 3 years ago

Some necro-post here, but the issue is still in 'Opened' status, and this message may be useful for some developers' categories.

LazyServiceIdentifier absolutely does no make sense for resolving circular dependency in the end. It is just a wrapper for your dependency, and when the container will try to resolve the dependencies tree it will fall into endless recursive resolving. I have no idea why, but even if every entity will have Singleton scope, entities will not be cached in resolving context during resolving of dependencies tree. The only available solution is using lazy loading with inject decorators lib. https://github.com/inversify/inversify-inject-decorators However it must be said, that lazyLoading will create a new request for the container (how I can understand it). I can not say how it will affect your code performance or code behavior, but it makes the 'Request' scope feature a little bit useless because interaction with lazy-loaded dependency will perform one more request to the container (just look into sources).

Not for the topic, but still actually: You can not use property injection with LazyServiceIdentifier, just because unwrapping is missed for properties in the latest release. The issue #1177 with resolving this bug opened for 2 years, but there was no result.

TotooriaHyperion commented 2 years ago

LazyServiceIdentifier's purpose is to handle circular reference to 'ServiceIdentifier' instead of instance to be injected. It doesn't delay the inject behavior after all the instances have been constructed.

You can use @injectLazy, which delays the injection to when the property is accessed.

I know it can't be use as a global annotation which made this feature very limited.

My way to solve this problem is.

  1. to only use singleton scope and
  2. inject container to itself
  3. then use property getter to get the instance.

For example:

@injectable()
class A {
  @inject(Container)
  container: Container;

  get b() {
    return this.container.get(B);
  }
}
@injectable()
class B {
  @inject(Container)
  container: Container;

  get a() {
    return this.container.get(A);
  }
}

const container = new Container({ defaultScope: 'Singleton' });
container.bind(Container).toConstantValue(container);
container.bind(A).toSelf();
container.bind(B).toSelf();
// example of multiple instances.
const rootContainer = new Container({ defaultScope: 'Singleton' }); // holds shared singleton services

const customModule = new ContainerModule(bind => {
  bind(A).toSelf();
  bind(B).toSelf();
})

@injectable()
class A {
  @inject(Container)
  container: Container;

  static create(parentContainer: Container) {
    const container = new Container({ defaultScope: 'Singleton' });
    container.parent = parentContainer;
    container.bind(Container).toConstantValue(container);
    container.load(customModule);
  }

  get b() {
    return this.container.get(B);
  }
}
@injectable()
class B {
  @inject(Container)
  container: Container;

  get a() {
    return this.container.get(A);
  }
}

const a1 = A.create(rootContainer);
const a2 = A.create(rootContainer);
LucaSorvillo commented 8 months ago

Could you recommend another library/framework that handles this problem effectively? it is frustrating in 2024 to still struggle with these problems.

LucaSorvillo commented 8 months ago

Could you recommend another library/framework that handles this problem effectively? it is frustrating in 2024 to still struggle with these problems.

I am responding to myself and hope it is helpful to others. I have switched to TSyringe which handles circular dependencies excellently and is much easier to set up.

@inject(delay(() => MyServiceClass)) myService: MyServiceClass