Factory aware of target class that resolves the factory's type.

[perf2711]

This pull request enables factory for resolving dependencies to see which class is using the factory to resolve the dependency.

For example, when you use factory to resolve Logger class, which contains parameter name in its constructor, you can provide parent's class name to automatically name the logger.

Example of final usage:

export class Logger {
  constructor(private name: string) {}

  public info(message: string) {
    console.log(`[${this.name}] ${message}`);
  }
}

// New "target" argument in factory, which contains the constructor of parent class (or undefined, if resolved directly from container)
const loggerFactory: FactoryFunction<Logger> = (_, target) => new Logger(target ? target.name : 'Default');
container.register(Logger, { useFactory: loggerFactory });

@injectable()
export class TestClass {
  constructor(logger: Logger) {
    logger.info('test') // [TestClass] test
  }
}

[msftclas]

All CLA requirements met.

[asciidisco]

Is there anything a non-committer like me can do to help get this merged? It would help us a lot to get rid of some boilerplate code, not only as described in the logging example above, but also for injecting configuration into our classes.

[Xapphire13]

@asciidisco, hang tight; I will give this another look over this week. Thanks for your patients =]

[MeltingMosaic]

I think that passing a constructor to the method leaves room for weird abuses, so instead I think it might be better to pass metadata about the constructor instead - would that still work for your scenario? I've had Hyrum's Law on my mind a lot lately, and I'm trying to keep the surface area low.

[asciidisco]

I don't own neither the PR nor would I consider myself a domain expert in DI/IoC topics nor Typescript, so I don't know how much weight you should put on my thoughts.

Back in the day when I started as a developer in the web sphere, things like you mentioned with method leaves room for weird abuses were actually a good thing, as they made things possible that the domain owners of libraries & browser vendors couldn't forsee. Therefore I wouldn't consider it a bad thing per se, that the implementation that is presented in this PR could present more powers to the user. On the other side, I use tsyringe (and not one of the other DI libraries that are around) because of its narrowed scope & clean API.

Summed up, I'd be happy with the approach of passing just the metadata that @MeltingMosaic proposed, as it would enable the use cases I have.

I'd even go that far & say that the metadata approach would not limit the proposed functionality to be used in factory functions only. From my point of view, it would be an interesting approach to inject the metadata into constructor methods that especially "ask" for them, withiout using a factory.

I'm thinking of something like this:

import { autoInjectable, SomeMagicMetadataInterface } from 'tsyringe'

@autoInjectable()
export class Logger {
  private name: string
  constructor({ name }: SomeMagicMetadataInterface) {
    this.name = name
  }

  public info(message: string) {
    console.log(`[${this.name}] ${message}`);
  }
}

@autoInjectable()
export class TestClass {
  constructor(private logger: Logger) {
    logger.info('test') // [TestClass] test
  }
}

Don't know if this is a bit far fetched, but it could potentially reduce boilerplate code a bit further.

[Xapphire13]

I'm still a little torn on which approach is best for this; I can see on one hand that a metadata object is just the right amount of context that the problem needs, but on the other do see how passing the target itself could enable some pretty cool stuff in the right hands.

I'm thinking maybe for now to go the route of the metadata object, starting with what we need (the targetName). And if in the future we need more (such as targetConstructor), then we can add as needed as a non-breaking change.

With regards to your idea of allowing the metadata object be injected into a class, I'm not so sure I would want to allow this; I can see this leading to code that's written in a super tight-coupled way to the DI framework, when the whole point of DI is to promote loose-coupling.

[Xapphire13]

@perf2711, are you still working on this?

[perf2711]

@Xapphire13 sorry, no, not at the moment.