tsyringe icon indicating copy to clipboard operation
tsyringe copied to clipboard

Child container doesn't auto inject from same container for ContainerScoped classes

Open crystalin opened this issue 5 years ago • 7 comments

Describe the bug When using a child container, when resolving a class (with Lifecycle.ContainerScoped) its auto injected classes (also Lifecycle.ContainerScoped) are not taken from the child container.

To Reproduce

import { autoInjectable, container, Lifecycle, scoped } from "tsyringe";

@scoped(Lifecycle.ContainerScoped)
export class A {}

@scoped(Lifecycle.ContainerScoped)
@autoInjectable()
export class B {
  constructor(public a: A) {}
}

const childContainer = container.createChildContainer();
const a = childContainer.resolve(A);
const b = childContainer.resolve(B);

console.log(a === b.a); // returns false, expecting true

Expected behavior the auto injected class should be resolved from the same container

Version: 4.0.1

crystalin avatar Dec 06 '19 19:12 crystalin

Actually, I just saw that the autoInjectable is specifying it is using the globalContainer. Is there a way to make it support local container ?

crystalin avatar Dec 06 '19 19:12 crystalin

Is there a way to make it support local container ?

I'll have to think about it a bit, but I don't think so. autoInjectable only knows about the global container at compile-time, and not the child containers which are created at runtime.

If this is something really needed, I can think of a few ways to support it, but nothing concrete right now.

MeltingMosaic avatar Dec 07 '19 19:12 MeltingMosaic

Maybe add a container parameter to the @autoInjectable() decorator? If specified the decorator will use it to resolve the dependencies.

import { autoInjectable, container, Lifecycle, scoped } from "tsyringe";

const childContainer = container.createChildContainer();

@scoped(Lifecycle.ContainerScoped)
export class A {}

@scoped(Lifecycle.ContainerScoped)
@autoInjectable(childContainer)
export class B {
  constructor(public a: A) {}
}

const a = childContainer.resolve(A);
const b = new B();

console.log(a === b.a);

skiptirengu avatar Dec 08 '19 01:12 skiptirengu

@crystalin Could you outline a use case for the behaviour?

The way I interpret @skiptirengu's code snippet is kind of like module level resolution

Xapphire13 avatar Dec 09 '19 07:12 Xapphire13

@Xapphire13 here is a use case: I have multiple classes with @scoped(Lifecycle.ContainerScoped) because I want them to act as singletons. Ex:

  • GitHubService which is responsible for querying github
  • UploaderService which is responsible for uploading images and uses the GitHubService

The UploaderService is defined like this

@scoped(Lifecycle.ContainerScoped)
@autoInjectable()
class UploaderService extends Service {
  constructor(private githubService: GitHubService){
    super("uploader");
  }
  ...
}

It works perfectly when using the service like this, but it becomes an issue when running the tests. The goal is to have each test its own container, allowing them to run independently even when stubbing the methods of those Services. (Those tests are running concurrently)

2) Broken solution Another approach I tried was to replace the @autoInjectable() by @inject(..) directly in the constructor. That solve the current problem but brings another one:

  • When the class extends another one, the parent doesn't automatically inject the missing parameters.

Ex:

@scoped(Lifecycle.ContainerScoped)
class Service {
  constructor(private name: string, @inject(HttpConfig) public httpConfig: HttpConfig){}
  ...
}

@scoped(Lifecycle.ContainerScoped)
class UploaderService extends Service {
  constructor(@inject(GitHubService) private githubService: GitHubService){
    super("uploader");
  }
  ...
}

const uploader = childContainer.resolve(UploaderService);
assert(!!uploader.githubService); // this is true
assert(!!uploader.httpConfig); // this is false :(

When executing the UploaderService constructor, it doesn't inject the httpConfig from the container.

crystalin avatar Dec 09 '19 13:12 crystalin

@skiptirengu solution wouldn't work in my case, as those container are resolved at run-time, after loading the module. Maybe loading the module multiple times with a different container each time, but I'm not sure how easy it would be

crystalin avatar Dec 09 '19 13:12 crystalin

Thanks for the use case, I'll sync up with @MeltingMosaic and we'll give it a think over

Xapphire13 avatar Dec 22 '19 06:12 Xapphire13