Allow injecting multiple providers for a single injector (closes #1083)

[matthew-salerno]

Overview

Closes #1083, it is unfortunately more of a rewrite than I'm comfortable with, but I'll explain my thought process.

Support for injectAll was first added in PR #1164, however it lacked the ability to provide multiple providers under the same token for the same inject, e.g.:

it("should allow getting multiple providers for the same token on the same injector", () => {
  const TOKEN = new StaticToken<string>("TOKEN");

  const injector = new Injector({
    providers: [
      [TOKEN, { factory: () => "first" }],
      [TOKEN, { factory: () => "second" }],
      [TOKEN, { factory: () => "third" }],
      [TOKEN, { factory: () => "fourth" }],
    ],
  });

  const res = injector.injectAll(TOKEN);

  assert.deepEqual(res, ["first", "second", "third", "fourth"]);
});

With this pull request, the above test case passes.

Breaking changes

Support for multiple providers for the same token on the same instance required deep changes to the underlying implementation to keep behavior working as it has with minimal breaks. However, not all breaks were avoided. This adds three breaking changes to the interface (that I have caught and decided not to fix):

1. The providers property no longer exists on injectors. Providers offer a great interface for describing providers, but it is easier internally to convert everything directly to instances and factories. Therefore, provider information is not stored.
2. The error: Provider for Service found but is missing either 'use', 'factory', or 'value' was changed to
   Provider for Service is missing 'use', 'factory', or 'value', and it now is thrown on injector construction instead of when it is accessed. I consider this an improvement as it alerts developers to improper usage of the API earlier on.
3. When using injectAll, the injector will not add an instance from its default factory (that is, a StaticToken's factory or a ConstructableToken's constructor) if no previous instances or providers have been found. Under the current logic (before this PR), what would happen is if a provider existed in a child, and not in a parent, an instance would be created in the parent and the returned length would be two. Now it would just be one, only returning the entry on the child, unless provided explicitly in the parent. This effectively means that inject(A) will return an instance of A, whereas injectAll(A) would return [], unless A is explicitly provided. This is to sidestep the hard question of which injectors one would expect to have added when injecting all, and how to determine whether or not one has already been provided by default previously. In general I think this is acceptable since most (all?) uses of injectAll don't need the convenience of automatically registering a new provider for classes and static tokens.

Surprising outcomes

Duplicated Factories

See the below test:

it("should inject the default factory for tokens when using multiple providers", () => {
  const TOKEN = new StaticToken<string>("TOKEN", () => "default");

  const injector = new Injector({
    providers: [
      [TOKEN, { factory: () => "first" }],
      [TOKEN, { factory: () => "second" }],
      [TOKEN, { factory: () => "third" }],
      [TOKEN, { factory: () => "fourth" }],
    ],
  });

  const res = injector.injectAll(TOKEN);

  assert.deepEqual(res, [
    "first",
    "second",
    "third",
    "fourth",
    "default",
    "default",
    "default",
    "default",
  ]);
});

This is a little surprising, and I mentioned deduplicating this before, alternatively it may be better not to add the default at all when providing, that behavior in and of itself is a little surprising IMO. I'm not sure which is preferable. Adding deduplication logic shouldn't be too hard but it is more lines of code. I would propose removing the behavior that providing a token also provides it's default factory entirely, but it would be another breaking change:

const TOKEN = new StaticToken<string>("TOKEN", () => "default");

const injector = new Injector({
  providers: [
    [TOKEN, { factory: () => "first" }],
    [TOKEN, { factory: () => "second" }],
    [TOKEN, { factory: () => "third" }],
    [TOKEN, { factory: () => "fourth" }],
    [TOKEN, { use: TOKEN}]
  ],
});

const res = injector.injectAll(TOKEN);

assert.deepEqual(res, ["first", "second", "third", "fourth", "default"]);

and

const TOKEN = new StaticToken<string>("TOKEN", () => "default");

const injector = new Injector({
  providers: [
    [TOKEN, { factory: () => "first" }],
    [TOKEN, { factory: () => "second" }],
    [TOKEN, { factory: () => "third" }],
    [TOKEN, { factory: () => "fourth" }],
  ],
});

const res = injector.injectAll(TOKEN);

assert.deepEqual(res, ["first", "second", "third", "fourth"]);

We could make it more ergonomic and have TOKEN be a shorthand for [TOKEN, {use: TOKEN}].

Design

Current

The current (before this PR) injection system relied on a map to cache instances for tokens. This limited the number of instances and providers for a token to one-per-instance. Multi injection was achieved through continuing to call up the parent injectors. This method was reliable and robust. Simply changing the cache to a map to an array of tokens was attempted, but had unforeseen consequences in the ordering of cache entries.

Cache system

The proposed design instead adds a cache system for injectors. Each injector has a cache, a cache contains a map from tokens to cache-rows. Each cache-row contains a list of cache-entries, ordered by priority (so the ordering when using multiple overrides is maintained). A cache-entry contains a factory and cached instance if available, the associated token, it's priority, and the injector that owns the cache that the cache entry belongs to.

export class InjectorCacheEntry<T> {
  get factory() {
    return this.#_factory;
  }
  get priority() {
    return this.#_priority;
  }
  get instance() {
    return this.#_instance;
  }
  [INJECTOR]: Injector;
  #_instance?: T | undefined;
  #_priority: InjectorCacheEntryPriority;
  #_factory?: ProviderFactory<T> | undefined;
  #token: InjectionToken<T>;

This additional information allows instantiation logic to be handled on the cache-entry instead of by the injector, this in turn allows objects other than the associated injector to handle instantiation. This seems like an anti-pattern, but it drives the core of the iterator-based injection system.

Iterator system

The iterator based injector system trades robust simplicity for being highly flexible. Injectors now have an #iter private method, which allows them to iterate over all cache-entries. Caches are now exposed as protected members to allow this. While this does remove some safety by revealing internals of injectors to other injectors, it allows for highly customizable traversal through the injection hierarchy. The current implementation of #iter searches up the tree once, for each injector, the order is:

1. Directly injected values, such as injectors or provideSelfAs
2. Entries provided in the providers constructor option
3. Static Tokens
4. Constructable Tokens

This order ensures user provided overrides are returned before default constructors and static tokens. I suppose it could be 2 tiers instead of 4.

This iterator driven approach means that inject and injectAll can have the same implementations, with the only difference being how far the iterator goes. In inject, we just return the first result from the iterator. In injectAll, we return all results.

Tradeoffs

Priority system feels arbitrary

This was largely implemented to default implementations to the back of the results. If we implemented the changes in Duplicated Factories, we may be able to remove the ordering system of cache rows.

Much more complex

The imperative design focused around iterators comes with a lot of added technical debt. While I tried to make it as understandable as I could, it is undeniably more complex and harder to reason about over the current system.

Likely more resource intensive

I suspect memory and code size are both impacted negatively, performance is likely slightly slower but similar. I haven't ran any benchmarks though.

Introduction of a protected member

the _cache member is protected, which is only enforceable in TypeScript and not transpiled JavaScript. Currently most of the code bases uses public members or members made private by #, this marks a departure from that norm.

Benefits

More customizable

This opens the door to more advanced features in the future, such as filtering out particular subclasses or allowing arbitrary skips or stopping points (not that this is necessarily advisable, I just think it's cool). An earlier version first traversed the tree looking for instantiated instances, and then again for uninstantiated providers, but it was unnecessary.

Can walk the tree without injecting

This also might pave the way for #960 since the injector tree is now traversible.

Multiple providers with the same token can be provided to one injector

The original motivation behind this PR.

Testing

Unit tests for injector.ts were expanded. injector-cache.ts has no test file because:

• It's more of an extension of the injector logic than something meant to be used independently.
• It requires an injector for construction, so we'd have to construct an injector and then reach in to the protected cache to test directly.
• What we're concerned with is whether the injector behaves correctly, the injector-cache just exists to support that.
• It already has 98% coverage because it is exercised via the injector tests.
• Many of the existing injector tests would become redundant, since the logic they is handled has moved almost entirely to the injector-cache.

[deebloo]

@matthew-salerno hey thanks for the PR! This is definitely something I want to get working. I will review your PR when I get a chance. I appreciate all of the thought that went into this as well. I have attempted several solutions but always ran into issues at some point. (that also explains how I ended up at my current simple tree traversal method)