Add name and tags to ResolutionContext

[BurningEnlightenment]

Is there an existing proposal similar to this?

• I have searched the existing proposals

What are you proposing?

Expose name and tags on the ResolutionContext interface like they are exposed on BindingConstraints.

Is there any specific group of users that will benefit from this?

Currently we have a custom decorator which combines @inject() and @tagged() to annotate a specific service with metadata which will be used during the resolution / construction of the service.

What problems are you trying to solve?

We want to inject logger instances with categories assigned into our services like so @logger('log.category').

With inversify v6 we were able to extract the tag from within toDynamicValue() and pass it to the logger.

Do you have any references or examples that can illustrate your idea?

ah sketch of the current v6 code:

export function logger(category: string): ParameterDecorator {
  const tagCategory = tagged(LogCategoryKey, category);
  const injectLogger = inject(Logger);

  return function (target, propertyKey, parameterIndex) {
    tagCategory(target, propertyKey, parameterIndex);
    injectLogger(target, propertyKey, parameterIndex);
  };
}

class MyService {
  constructor(@logger('my.category') private readonly log: Logger) {}
}

container.bind(Logger)
    .toDynamicValue((ctx) => {
      const category = ctx.currentRequest.target
        .getCustomTags()
        ?.find(({ key }) => key === LogCategoryKey);

      /*…validation…*/

      const rootLogger = ctx.container.get(RootLogger);
      return rootLogger.child({ category: category.value });
    })
    .inTransientScope();

What type of idea is this?

Copy of existing idea: Similar idea exists and this is a copy

[notaphplover]

Hey @BurningEnlightenment, thank you for submitting the feature request.

At first glance, names and tags are scoped to be used for injection purposes in the planning phase of the container. Not providing such metadata in the resolve phase is done on purpose. I understand your use case but I believe knowing binding constraints is not required to provide an equivalent implementation.

Implementation proposal

Since categories are a well known set of constants, consider the following approach:

container.bind(Logger)
  .toResolvedValue(
    (rootLogger: RootLogger) => rootLogger.child({ category: 'my.category' }),
    [RootLogger],
  )
  .whenTagged(LogCategoryKey, 'my.category');

This binding is able to cover your use case for that specific category. Extracting a function:

function bindCategoryLogger(container: Container, category: string): void {
  container.bind(Logger)
    .toResolvedValue(
      (rootLogger: RootLogger) => rootLogger.child({ category }),
      [RootLogger],
    )
    .whenTagged(LogCategoryKey, category);
}

Once we got the function, we can add bindings for each category:

for (const category of MY_WELL_KNOWN_CATEGORIES) {
  bindCategoryLogger(container, category);
}

What about the feature request?

We should ask if the public API should expose planning related data such as binding constraints. I honestly prefer not do so. Constrained bindings should be the way to determine which service shall be provided as I previously proposed with bindCategoryLogger. I understand the potential of this feature request, it enhances ResolutionContext based bindings, but this enhancement imho has a big downside: we are sharing the responsibility of planning with the planner, which is not my cup of tea.

At the end, the dynamic value you want to implement decides which logger should be relied on given a set of binding constraints, this responsibility belongs to the planner instead. As long as the binding API is powerfull enough to satisfy these kind of use cases, that should be the way to go.

I'm keeping the issue open for the sake of discussion, I'd love to hear anyone's thoughts about this.

[BurningEnlightenment]

The log categories are usually specific to a service and I did consider your approach. Though I hoped I could avoid the tedium of specifying the category twice without implementing some global category registry like the following myself.

const MY_WELL_KNOWN_CATEGORIES = new Set<string>();
export function logger(category: string): ParameterDecorator {
  MY_WELL_KNOWN_CATEGORIES.add(category);

  const tagCategory = tagged(LogCategoryKey, category);
  const injectLogger = inject(Logger);

  return function (target, propertyKey, parameterIndex) {
    tagCategory(target, propertyKey, parameterIndex);
    injectLogger(target, propertyKey, parameterIndex);
  };
}

It is trivial to do, but it feels weird that the planning complexity for a logger has to scale linearly with the number of log categories by design. I took a cursory glance at the binding constraint implementation, it doesn't seem to (be able to) cache plans based on constraints, i.e. assuming all categories are unique we've increased runtime complexity for logger construction from O(n) (or maybe O(n*log n) if child() caches instances based on category) to O(n^2).

So it is probably more efficient to dynamically generate injection symbols for each log category…

const MY_WELL_KNOWN_CATEGORIES = new Map<string, symbol>();
export function logger(category: string): ParameterDecorator {
  let loggerServiceId = MY_WELL_KNOWN_CATEGORIES.get(category)
  if (loggerServiceId == null) {
    loggerServiceId = Symbol(`logger#${category}`);
    MY_WELL_KNOWN_CATEGORIES.set(category, loggerServiceId);
  }
  return inject(loggerServiceId)
}

function bindCategoryLogger(container: Container, category: string, loggerId: symbol): void {
  container.bind(loggerId)
    .toResolvedValue(
      (rootLogger: RootLogger) => rootLogger.child({ category }),
      [RootLogger],
    );
}

for (const [category, loggerId] of MY_WELL_KNOWN_CATEGORIES) {
  bindCategoryLogger(container, category, loggerId);
}

I opened this issue, because this feels like I'm hacking around an API deficiency, but if it is the intended way to use inversify, so be it.

Obviously any problem can be solved with an additional layer of indirection, so another alternative would be to just inject a factory function which wraps child() and forwards the category. However, this adds noise to the services as they have to call the factory function in their constructor.

[notaphplover]

It is trivial to do, but it feels weird that the planning complexity for a logger has to scale linearly with the number of log categories by design. I took a cursory glance at the binding constraint implementation, it doesn't seem to (be able to) cache plans based on constraints, i.e. assuming all categories are unique we've increased runtime complexity for logger construction from O(n) (or maybe O(n*log n) if child() caches instances based on category) to O(n^2).

Well, we should clarify what is n here and I honeslty believe the computational cost depends on a variety of factors we are not keeping in mind, like the amount of different root plans and the maximum amount of nodes related to a root plan. When you add those variables to the computational cost, I don't think the complexity is better. Unless you're only building Loggers in your app, the complexity should be the same because the real multiplier is not going to be the amount of different categories, but the amount of different root requests instead, which is going to be applied no matter what. The big O is not trivial at all to calculate, and it's not easy to know when n is big enough to make O(f(n)) relevant.

I opened this issue, because this feels like I'm hacking around an API deficiency, but if it is the intended way to use inversify, so be it.

I don't see it that way, but I'm interested to know more about it. What feels hacky? Don't worry about being direct, I strongly believe in constructive criticism as a way of improving 😄.

[BurningEnlightenment]

Well, we should clarify what is n here and I honeslty believe the computational cost depends on a variety of variables we are not keeping in mind, like the amount of different root plans and the maximum amount of nodes related to a root plan. When you add those variables to the computational cost, I don't think the complexity is better. Unless you're only building Loggers in your app, the complexity should be the same because the real multiplier is not going to be the amount of different categories, but the amount of different root requests instead.

First off: I'm sometimes a bit… obsessed with removing overhead and communicate it bit too strongly. I really just wanted to express that this realization made me feel weird. I didn't want to suggest it dominating the overall DI runtime or even causing noticable delays. With that out of the way I was talking about the worst case of only injecting loggers with unique categories with n being the number of categories. Keep in mind that O-complexity is only a lower bound and as you pointed out the overall DI algorithm complexity will probably be worse anyways due to other transitive properties and n will probably never be big enough to matter anyways.

[BurningEnlightenment]

I don't see it that way, but I'm interested to know more about it. What feels hacky?

It feels like I'm implementing some sort of reflection capability with a global registry. I fully agree that toDynamicValue() should not take on the planner role and switch between different implementations. Usually with DI one just says I want X and the IoC takes care of fulfilling that request. Whereas in the logger category case, there is no X but a flavor of X which I want. Sort of like generics in most languages allow you not only to create e.g. a List but a List<string>. So maybe the generalization of this use case would be to not only support injecting X but also X<T> whereas T would be my logging category.