Named bindings

[murat-mehmet]

In Inversify I was using named bindings for many useful things. Is there an alternative here or any plan to implement it?

[mscharley]

There is nothing currently, but this is the proposal that was supposed to help deal with this sort of situation. In truth, I've never needed named bindings so I'm not 100% across what the needs behind it are though. Is there anything in that proposal which would be missing for your usecase?

[mscharley]

My concern is that I don't think this meets the spirit of your first example which seems to be allowing the use of arbitrarily named loggers. Only requiring metadata for factory bindings which would be cached might be one solution there I guess.

One other feature that dot has which Inversify doesn't is container validation. I don't think this would be impossible, but the number of special cases here could make the validation logic harder to write and maintain. There's already some gaps in the validation algorithm and I'd rather not make that gap wider.

[murat-mehmet]

Sorry for the confusion. I think we are talking about the same thing. Let me clarify what I meant with further examples.

The following example matches ur latest assertions.

// Example 1
container.bind(LOGGER).inSingletonScope().withMetadata({ name: "CONN" }).toFactory(({ metadata }) => new LoggerService(metadata.name);
container.bind(LOGGER).inSingletonScope().withMetadata({ name: "HELLO" }).toFactory(({ metadata }) => new LoggerService(metadata.name));

const connLogger = container.get(LoggerService, { name: "CONN" });
const helloLogger = container.get(LoggerService, { name: "HELLO" });

expect(connLogger).not.toBe(helloLogger); // true <- Two different singletons
const l2 = container.get(LoggerService); // Exception: ambiguous binding found, multiple bindings match this request but a single element was requested.
const loggers = container.get(withOptions(LoggerService, { multiple: true}));
expect(loggers).toBe([connLogger, helloLogger]);

The following example is the one I intended to present.

// Example 2
// Singleton with NO withMetadata
container.bind(LOGGER).inSingletonScope().toFactory(({ metadata }) => new LoggerService(metadata.name);

const connLogger = container.get(LoggerService, { name: "CONN" });
const helloLogger = container.get(LoggerService, { name: "HELLO" });

expect(connLogger).not.toBe(helloLogger); // FALSE <- One singleton would be cached at first call to .get() 
expect(helloLogger.prefix).toBe('CONN') // TRUE <- because the first call was made with CONN

const l2 = container.get(LoggerService); // Won't throw an exception.
const loggers = container.get(withOptions(LoggerService, { multiple: true})); // This could just return the one singleton.
expect(loggers).toBe([connLogger]);

The example 2 is useless of course, this kind of LoggerService makes sense only in transient scope.

You would cache metadata only when withMetadata is specified in bindings as in Example 1. I think this is also not against your use case, right?

[mscharley]

I'm inclined to force withMetadata() for all bindings except one outlier - transient factory bindings.

In all situations other than factory bindings, metadata is necessary to distinguish between different bound values, so must be provided. Factorys however are in the unique position of being able to dynamically return a different value based on the metadata provided.

Caching factory bindings only makes sense when you have explicit metadata for the binding. Take for example the following:

container.bind(LOGGER).inSingletonScope().toFactory(({ metadata }) => new LoggerService(metadata.name));

@injectable(withMetadata(LOGGER, { name: "A-LOGGER" }))
class A {
  public constructor(private logger: LOGGER) { }
}

@injectable(withMetadata(LOGGER, { name: "B-LOGGER" }))
class B {
  public constructor(private logger: LOGGER) { }
}

@injectable(A, B)
class C {
  public constructor(private a: A, private b: B) { }
}

const c = container.get(C);
c.a.logger.name // What is this value? "A-LOGGER" or "B-LOGGER"?

The answer is that this is deterministic in dot for any given configuration thanks to the query planner, but it is brittle and will easily change with very small changes to the dependency tree.

I'm inclined to make the following rules around this:

// These are allowed:
container.bind(LOGGER).inSingletonScope().withMetadata({ name: "CONN" }).toFactory(({ metadata }) => new LoggerService(metadata.name);
container.bind(LOGGER).inTransientScope().withMetadata({ name: "CONN" }).toFactory(({ metadata }) => new LoggerService(metadata.name);
container.bind(LOGGER).inTransientScope().toFactory(({ metadata }) => new LoggerService(metadata.name));

// These might be allowed in the future, but would rely on smarter caching that can tell metadata input apart rather than caching purely based on the binding. This would mean that A-LOGGER and B-LOGGER above would both be cached separately.
container.bind(LOGGER).inSingletonScope().toFactory(({ metadata }) => new LoggerService(metadata.name));

// These are definitely errors.
container.bind(LOGGER).toConstantValue(new LoggerService("CONN"));
container.bind(LOGGER).inTransientScope().toDynamicValue([], () => new LoggerService("CONN"));

This still leaves one outstanding question that I just thought of. What would the following example do?

container.bind(LOGGER).inTransientScope().toFactory(({ metadata }) => new LoggerService(metadata.name));

const loggers = container.get(withOptions(LOGGER, { multiple: true, optional: true }, {}));

// Option A:
expect(loggers).toBe([new LoggerService(undefined)]);
// Option B:
expect(loggers).toBe([]);
// Option C:
expect(loggers).toThrow(new Error("Ambiguous binding, the only binding available for a multiple injection was a factory binding"));

I think option C might make more sense, this feels like an edge case on an edge case and something that doesn't make a whole lot of sense to attempt. On one hand you've setup the bindings to allow for arbitrary creation of objects based on input, but on the other you're trying to fetch a list of existing objects.

[murat-mehmet]

I'm inclined to force withMetadata() for all bindings except one outlier - transient factory bindings.

That makes sense and covers all the use cases I can think of.

This still leaves one outstanding question that I just thought of. What would the following example do?

Option A and C both seem ok. Option C looks a bit strict though. You have specified optional: true which I would expect it to prevent throwing any exception here. In this case I would expect it to unwrap like option A. Omitting optional could throw like in option C.

[mscharley]

The optional option is really about allowing the situation where there are no bindings at all rather than about suppressing errors. I think this is probably a legitimate error case, the only binding available is not one that makes sense which sounds like a configuration issue that probably shouldn't be hidden away.

This version of optional is most often useful in situations where you're writing infrastructure code that loads list of some kind of interface and operates over lots of them. Think, the queue manager I mentioned above or a HTTP server loading routes. These use cases definitely care about invalid bindings and wouldn't want to silently continue in a potentially ill defined situation but don't want to fail just because the user hasn't provided any content for them yet and perhaps won't ever.