glimmerjs / glimmer-di

MIT License
41 stars 9 forks source link

Proposal: Remove registration options entirely #23

Closed dgeb closed 7 years ago

dgeb commented 7 years ago

Currently there are two registration options:

export interface RegistrationOptions {
  singleton?: boolean;
  instantiate?: boolean;
}

If we move ahead with https://github.com/glimmerjs/glimmer-di/issues/21 then we can eliminate the instantiate option.

In order to remove the singleton option, we will require registration of factories that provide singleton construction behavior. In other words, Factory#create will need to return the same instance every time. This implies that factories can and should maintain state (frankly, the "can" part is a given with any create function).

mixonic commented 7 years ago

eliminating instantiate seems only tied to allowing the factory and class to be registered separately, I think. singleton definitely requires #21 though.

Is there a proposal for register(specifier, klass, factory, options?)?

I'm in favor of this if we split class/factory and implement #21.

dgeb commented 7 years ago

Is there a proposal for register(specifier, klass, factory, options?)?

I wanted to make this proposal first because removing options clears the way for:

register(specifier, class, creator = class, destructor?)

And a corresponding Factory interface:

interface Factory<T> {
  class: Constructor<T>;
  creator: Creator<T>;
  destructor?: Destructor<T>;

  // invokes `creator.create()` with registered injections
  create(injections?: Object): T;
}

Note: if we choose to separate class from creator, it will require further discussions and interface design.

dgeb commented 7 years ago

I am backing out of this proposal on further consideration. Singletons must be per-container, not per-registry, so it is inappropriate to use a factory registered in a registry to maintain state per-container.