RFC: Enhancements for Overriding Registrations in Aurelia DI Container

[Vheissu]

RFC: Enhancements for Overriding Registrations in Aurelia DI Container

[Sayan751]

@Vheissu Thanks for the clarification and providing the example use cases. For handling configuration options, there might be an easier way of dealing with configuration options. The following example is inspired by the options pattern in .netcore.

The idea is that there is one configuration source (can be more, though) and there is a configuration manager. This looks like below:

// this is loosely the schema of the configuration source, say config.json
interface ConfigurationOptions {
  features: string[]; // turn this into an array of literal type if that is what you needed
}

export const IConfigurationManager = DI.createInterface<IConfigurationManager>('IConfigurationManager', x => x.singleton(ConfigurationManager));
export type IConfigurationManager = ConfigurationManager;

export class ConfigurationManager<TConfig extends ConfigurationOptions = ConfigurationOptions> {
  private _systemConfigs: TConfig | undefined = undefined;
  
  public setConfiguration(systemConfigs: TConfig): void {
    systemConfigs.features = systemConfigs.features ?? [];
    this._systemConfigs = systemConfigs;
  }

  public isFeatureEnabled(featureName: string): boolean {
    return features.includes(featureName); // cache the result if you want
  }
}

Then you can simply inject the IConfigurationManager in your class and check if a certain feature flag is enabled or not. IN that way, you don't need to override any container registrations from tests, and you can rather provide the ConfigurationManager#setConfiguration with a different set of configuration options. Starting from this and creating a TC like with-feature="flagone" might be a trivial step.

<!-- the element-one will only be rendered if the flagone feature is on.
<element-one with-feature="flagone"></element-one>

Implementing the IContainer#deregister for this purpose seems like an overkill TBH. On a different note, this might be a good opportunity to implement something like the options pattern in .netcore for Au2, to offer something standard to manage application configuration options. 😉

[Sayan751]

Thanks @Vheissu for drafting the detailed RFC!
I have question regarding the enhancement 1 and 3. From the examples provided, the difference is not clear to me. Can you elaborate on that?

My personal opinion is that we opt for the 2nd alternative. Replacing a registered component in a DI, should be a conscious decision due to its potentially disruptive/destructive nature. Hence, I think that it should not be made extra user-friendly.

On a different note, I got a bit stuck by this:

The need to override dependencies arises primarily in tests, where it might be desirable to stub/mock dependencies.

So far, the existing DI infrastructure worked pretty well for me to supply those overrides from the tests. Would you mind sharing some example code to demonstrate where exactly the existing infra lacks?

[Vheissu]

@Sayan751 That was my bad on the example. I missed the deregister method. Enhancement 3 proposes implementing a deregister method and supporting a configurable override: true flag.

Enhancement 2 is where I am naturally leading, too. It would be the least destructive change to DI. And it would be intentional what deregister does, especially if someone uses it and wonders why dependencies are not present.

There is a workaround currently where you can use getAll or all and I added in a last resolver which will get the last dependency if there are multiple registered under the same key. However, this arose from a use-case that goes something like this:

Say you have feature flags as enums:

export enum FeatureFlags {
  flagOne = 'valueone',
  flagTwo = 'valuetwo',
  flagThree = 'valuethree',
}

You then register those feature flags inside of some bootstrap code:

container.register(Registration.instance(FeatureFlags.flagOne, false));

You can inject that feature flag in your code:

static inject = [FeatureFlags.flagOne]

Inside your test you want to override the value:

bootstrap(() => {
    container.register(Registration.instance(FeatureFlags.flagOne, true));
});

This will add the value, not override the original feature flag value. Because it's the same container (bootstrap logic stuff is happening outside of the test and abstracted away because there's some globally repeated test stuff).

[bigopon]

Something like a flag/feature manager sounds like an excellence built in 💯 , we can have it in the addons package, or core is also fine too.

For the ability to undo a resolver, I quite agree that it is an overkill for this scenario, though there' cases where we need it in the framework ourselves, for example, hot reload where we also need to remove the old resource resolver to register the new one. That said, we don't have to do it now if we want to implement the suggestion from @Sayan751 .

[Sayan751]

My vote is to add the options pattern to the kernel package. As it is expected to be a low level infra, although it will be a public API. The advantage will be that inside the core package we can use that to better transport the configuration options in different parts of the core components.

[bigopon]

Resolved by #1981