angular/di.js

"Configuration" in di.js

caitp opened this issue · 24 comments

caitp commented

Really just moving a question from angular/http here, https://github.com/angular/http/pull/26#issuecomment-46907522 --- I think we want to be able to have a configuration pass before the runtime of the application, as in angular 1.x, and my question is really just "how does that work?"

This has been talked about in design docs, but I haven't gotten the chance to do it yet in di.js, so I would like some pointers on the right approach to this.

(hint: I don't think requiring developers to create a parent injector with a bunch of values registered and injected into our service is really the right way to do it, that seems like a nightmare)

buchanae commented

I was just experimenting with this on a project (not angular) and since it's on my mind I'll braindump here.

I'm trying a loader class with config, providers, and dependencies attributes. Instances of this loader class are registered with a "manager" on app start, before the main injector is created.

Let me demonstrate:

class CarLoader {
  constructor() {
    this.providers = [CarBody];  // Say, if you wanted to override Body injectable in a child injector
    this.deps = [Wheels, Windows];
  }

  config(wheels: WheelConfig, windows: WindowConfig) {
    wheels.count = 4;
    windows.powerSource = windows.manualPower;
  }
}

var services = [CarLoader];
function start() {
  var injector = new Injector();
  services.forEach((service) => { injector.get(service); });
}
caitp commented

Yeah but the thing is, this kind of sucks.

I think we could implement something like this in the "base" angular package (whatever that ends up being), but it's probably something that di.js itself should support, so that non-angular packages can use it, and angular packages don't need to depend on a core package

In 1.x it's like this: there are 2 injectors, one "config" injector with providers, and one "instance" injector.

This model would work just fine in di.js --- with one problem --- we have child injectors literally all over the place, so how do we say "this needs to exist in the root injector, only the root injector has configuration blocks" --- or do we need to say that at all? At the very least, for the lifetime of an instance injector, all of its providers should be static and unchanging. That doesn't necessarily mean that there can only be a single config block for a root injector in the application, but it's not clear to me how you make that "work", or how it "should" work. Maybe there are already faculties for this, but I don't think I've seen them yet.

RGBboy commented

What about injecting a config into the HTTP service?

export class HTTPConfig {
  constructor() {
  }
}

@Inject(HTTPConfig)
export class HTTP {
  constructor(config) {
    // do config here
  }
}

Then when users need to config they provide their own version?

@Provide(HTTPConfig)
export class CustomHTTPConfig {
  constructor() {
  }
}
vojtajina commented

I'd like to keep the configuration declarative.
@RGBboy 's proposal is that direction.
Why is that not sufficient?
Can we put together a list of requirements / use cases?

Configuration by mutating some object is a terrible idea in my opinion.
We are doing it in Karma and it's super fragile.
We need something better.

The two phases we have in Angular 1.x (config, run) is not any better. It was just an after-thought, an attempt to fix the problem that we created by having imperative configuration by object mutation (all the providers method such as $location.html5Mode(true); etc) in the first place.

Alxandr commented

I agree that it should be declarative. And overridable in child injectors.

caitp commented

I think injecting an arbitrary symbol as a way of configuring an object is actually more confusing, because things have to work together in different places for that to work, which is awkward.

An explicit configuration method would be simpler, like

// Injector.prototype.config, I guess --- instance method for one injector,
// although really you should be able to do this before the injector is ever
// created
Injector#config('Http', {
  // declarative HTTP configuration properties
});

--- this would somehow automatically be injected into Http, and wouldn't require any explicit creation of a factory or value to inject.

Basically what I'm saying is, we need to add some sugar to this to make it suck less, because otherwise it's a bit intimidating

vojtajina commented

My thinking is: why should configuration be any different compare to other things - services?

Agree we need some syntactic sugar to make it easier.

@caitp Can you list requirements / use case you can think of?
Eg. we need to support "default config" - Http service will define some default configuration and a user can override only parts of it.

Alxandr commented

What could work would be to create a "special" config symbol, which where global for all of the things.
Say this symbol was named di.CONFIG (just for this example):

@Inject(di.CONFIG)
export class Http {
    constructor(config) {
    }
}

// other file; di config
var inj = new Injector();
inj.config(Http, {
    useJSONP: false
});
var http = inj.get(Http); // injects the useJSONP = false

Special handling of config, and having them be POJOs would allow for a few interesting things. First of you get a simple consistent way of configuring ANYTHING (even things that can't be configured, they'd just ignore the configuration, since it's not injectd).

Secondly, you could merge configs from the root injector and inwards. Meaning if I had a root injector that had useJSONP: false, and a child with rootPath: '/', with a grandchild having useJSONP: true, I'd get {useJSONP: true, rootPath: '/'} which I think could be really powerful. Though I have no idea of the performance cost. Adding special handling for the symbol di.CONFIG should be easy enough though.

caitp commented

I'm just brainstorming here, but:

  1. it should be possible to configure providers for an injector before that injector is ever instantiated
  2. when the injector is instantiated, configuration values should be merged with default configurations (if any) for each provider
  3. configuration for a child injector should inherit the configuration from the parent injector, unless the developer requests not to (to support isolation for modules). this isolation should be configurable per-provider, as well as per-injector (for extra sugar). It could be a parameter to the config sugar method, and as well as the injector constructor
  4. it should be possible to queue up multiple configurations in sequence, and they should be run sequentially (primarily helpful for testing, beforeEach vs before single test, etc)
  5. registering a default configuration for a provider should be as simple as assigning an object to some property (lets call the property name Symbol('defaultConfiguration') or something).

I think those are the big ones --- none of this depends on imperative configuration, but it does depend on some machinery being in place to support these requirements. The use cases that need to be satisfied are pretty self-evident:

  1. configure before the runtime of the application (before an injector is instantiated)
  2. configure during the runtime of the application (overwrite configuration for current injector)
  3. support deleting properties from a configuration, not just augmenting
  4. isolate configuration per-module (required or optional)
  5. configure per segment of the application (child routers with their own injector, etc)
  6. merge default configuration if present
  7. queue up configuration calls and run them in sequence when ready, useful for testing

/cc @EisenbergEffect / @jeffbcross / @tbosch

Alxandr commented

I'd argue that configuration before instansiation of di is outside the scope of di, and rather inside the scope of angular, or whatever bootstraps the di. Otherwise you have to resort to globals, with the possibility of collisions etc. The root injector of angular could easily be configurable pre-instansiation by having methods in angular that just tucks away the config until the di is instansiated, and at that point they feed them to the di.

vojtajina commented

On Wed, Jul 30, 2014 at 11:12 AM, Alxandr notifications@github.com wrote:

What could work would be to create a "special" config symbol, which
where global for all of the things.
Say this symbol was named di.CONFIG (just for this example):

@Inject(di.CONFIG)export class Http {
constructor(config) {
}}
// other file; di configvar inj = new Injector();inj.config(Http, {
useJSONP: false});

This is imperative config that mutates the injector. Thus you get into
exactly the same troubles that led us into config/run phase in 1.x:
Somebody can instantiate Http before you call injector.config() and
would't get the config value.

var http = inj.get(Http); // injects the useJSONP = false

Special handling of config, and having them be POJOs would allow for a few
interesting things. First of you get a simple consistent way of configuring
ANYTHING (even things that can't be configured, they'd just ignore the
configuration, since it's not injectd).

I don't understand this.
If config is just a regular binding (as any other service such as Http),
you can configure anything as well.
How is this better?

Secondly, you could merge configs from the root injector and inwards.

Meaning if I had a root injector that had useJSONP: false, and a child
with rootPath: '/', with a grandchild having useJSONP: true, I'd get {useJSONP:
true, rootPath: '/'} which I think could be really powerful. Though I
have no idea of the performance cost. Adding special handling for the
symbol di.CONFIG should be easy enough though.

One note: "merging" is not that simple. Will that be a shallow merge or
deep? I think you can't answer this question for all apps. I think it can
be app specific, based on the semantics of your config. Thus, DI should not
care about this.

—

Reply to this email directly or view it on GitHub
#56 (comment).

vojtajina commented

On Wed, Jul 30, 2014 at 11:15 AM, Caitlin Potter notifications@github.com
wrote:

I'm just brainstorming here, but:

  1. it should be possible to configure providers for an injector before
    that injector is ever instantiated

Yes.

  1. when the injector is instantiated, configuration values should be
    merged with default configurations (if any) for each provider

Agree that this needs to be possible. I'm not sure if DI should do the
actual merging.

  1. configuration for a child injector should inherit the configuration
    from the parent injector, unless the developer requests not to (to support
    isolation for modules). this isolation should be configurable per-provider,
    as well as per-injector (for extra sugar). It could be a parameter to the
    config sugar method, and as well as the injector constructor
  2. it should be possible to queue up multiple configurations in
    sequence, and they should be run sequentially (primarily helpful for
    testing, beforeEach vs before single test, etc)

Can you elaborate on this? I don't think I understand it...
The configuration needs to be defined in multiple places and eventually
merged, right?

  1. registering a default configuration for a provider should be as
    simple as assigning an object to some property (lets call the property name
    Symbol('defaultConfiguration') or something).

I think those are the big ones --- none of this depends on imperative
configuration, but it does depend on some machinery being in place to
support these requirements. The use cases that need to be satisfied are
pretty self-evident:

  1. configure before the runtime of the application (before an injector
    is instantiated)
  2. configure during the runtime of the application (overwrite
    configuration for current injector)
  3. support deleting properties from a configuration, not just
    augmenting
  4. isolate configuration per-module (required or optional)
  5. configure per segment of the application (child routers with their
    own injector, etc)
  6. merge default configuration if present
  7. queue up configuration calls and run them in sequence when ready,
    useful for testing

/cc @EisenbergEffect https://github.com/EisenbergEffect / @jeffbcross
https://github.com/jeffbcross / @tbosch https://github.com/tbosch

—
Reply to this email directly or view it on GitHub
#56 (comment).

caitp commented

The configuration needs to be defined in multiple places and eventually
merged, right?

I assume you're talking about the second point? (queue up multiple configurations in sequence)

This is primarily helpful for tests --- EG beforeEach(setupFirstConfigBlock) followed by it(setupAnotherConfigBlockCreateInjectorAndRunTests). The second configuration should predictably run after the first configuration, and should allow overriding properties of the original configuration.

If di.js isn't doing the merging itself, then the "overriding properties" thing is really application defined (but I think it would be useful to have an implementation in Di.js so that apps don't rewrite it all over the place, and it would be good to have the more commonly used case as the default).

Running in sequence is more or less a property of the test framework, so the rest isn't that important. But it would be good to make it easy.

Alxandr commented

Let me try and explain a bit better then.

The only real difference from my "special handling" of config (if we ignore the merging for now, and I'll get back to that later) and just having config as any old injectable binding is that you have 1 token/symbol (or whatever you want to call it) that can be used to inject config into anything. This would let people know that if they want to configure something, just create a "configuration" for "that thing" (example below). Whereas, if there is no enforced standard way to do this, different apps can have different way of configuration, that might confuse people.

Let's say I have 2 modules/classes that I want to inject into different places. Http and Log. Http is configurable, where as Log is not. Something like this:

@Inject(di.CONFIG)
export class Http {
    constructor(config) {
    }
    // methods
}

@Inject() // don't know how to write something that's injectable, but doesn't have any deps
export class Log {
    constructor(config) {
    }
    // methods
}

Now, the point about being able to inject Http before I've configured it is completely valid, and this also breaks with the immutable injectors principle (that I'm not even sure you went for or not). So rater, the config should (just as the providers) be given to the injector at construction time. So you have:

var conf = [];
conf.push(Http, {useJsonP: true});
conf.push(Log, {something: false});

var di = new Injector(conf, [Http, Log]);

Here I've added conf as an array of key-values. There are obviously better types, but this is just an sample to explain my idea. This effectively turns di into a two pass like in angular 1, because you have to have the config ready by the time you create an injector, with the exception that you can override configs in child-injectors should you so choose.

This also explains my point about how ANYTHING can be configured, because even though Log can't be configured, you can still provide a config object for it, though it's just ignored. This can obviously also be seen as a anti-point, as some might expect errors if config provides values that aren't used, though I think that for javascript this behavior is perfectly ok.

Now, as with regards to merging, I would do shallow merging, because that would probably cover the most common case of config. The reason being that if you need anything else you can actually do so yourself. Like this:

var conf1 = [Http, {useJsonP: false}];
var conf2 = [Http, {preventCache: true}];

var di1 = new Injector(conf1, [Http]);
var di2 = di1.createChildInjector(conf2); // at this point, merge happens. Not at lookup.

// if we want deep merge
var origConfig = di2.getConfig(Http); // or some other way to get the config
var newConfig = customMergeFunction(origConfig, changes);
var di3 = di2.createChildInjector(newConfig);

And if there's anything I left out to explain, please do ask :)

Alxandr commented

Oh, and as for usage in angular, I would say angular has some Config method that's called before bootstrapping happens, that builds up the config "array", and uses it when constructing the first (and root) di injector.

vojtajina commented

On Thu, Aug 7, 2014 at 11:44 AM, Alxandr notifications@github.com wrote:

Let me try and explain a bit better then.

The only real difference from my "special handling" of config (if we
ignore the merging for now, and I'll get back to that later) and just
having config as any old injectable binding is that you have 1 token/symbol
(or whatever you want to call it) that can be used to inject config into
anything. This would let people know that if they want to configure
something, just create a "configuration" for "that thing" (example below).
Whereas, if there is no enforced standard way to do this, different apps
can have different way of configuration, that might confuse people.

Let's say I have 2 modules/classes that I want to inject into different
places. Http and Log. Http is configurable, where as Log is not.
Something like this:

@Inject(di.CONFIG)export class Http {
constructor(config) {
}

// methods}

@Inject() // don't know how to write something that's injectable, but doesn't have any depsexport class Log {
constructor(config) {
}
// methods}

Now, the point about being able to inject Http before I've configured it
is completely valid, and this also breaks with the immutable injectors
principle (that I'm not even sure you went for or not). So rater, the
config should (just as the providers) be given to the injector at
construction time. So you have:

var conf = [];conf.push(Http, {useJsonP: true});conf.push(Log, {something: false});

Yes. This is exactly what I would do right now. You don't need any changes
to DI.

var di = new Injector(conf, [Http, Log]);

Here I've added conf as an array of key-values. There are obviously better
types, but this is just an sample to explain my idea. This effectively
turns di into a two pass like in angular 1, because you have to have the
config ready by the time you create an injector, with the exception that
you can override configs in child-injectors should you so choose.

This also explains my point about how ANYTHING can be configured,
because even though Log can't be configured, you can still provide a
config object for it, though it's just ignored. This can obviously also
be seen as a anti-point, as some might expect errors if config provides
values that aren't used, though I think that for javascript this behavior
is perfectly ok.

Now, as with regards to merging, I would do shallow merging, because that
would probably cover the most common case of config. The reason being that
if you need anything else you can actually do so yourself. Like this:

var conf1 = [Http, {useJsonP: false}];var conf2 = [Http, {preventCache: true}];
var di1 = new Injector(conf1, [Http]);var di2 = di1.createChildInjector(conf2); // at this point, merge happens. Not at lookup.

Why are you treating the config as something special?
Angular can do the merging (in a way it wants) and provide config in the
same way as anything else.

var di = new Injector([
  bind(Config).toValue(mergedConfig)
]);

// Config is a token, representing configuration. Defined by Angular.
// bind(...).to(...) is a helper to define providers (something we don't
have in DI right now but need anyway)

// if we want deep mergevar origConfig = di2.getConfig(Http); // or
some other way to get the configvar newConfig =
customMergeFunction(origConfig, changes);var di3 =
di2.createChildInjector(newConfig);
>
> And if there's anything I left out to explain, please do ask :)
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/angular/di.js/issues/56#issuecomment-51514247>.
>
Alxandr commented

But doesn't that mean you have 1 global (potentially HUGE) config object? And not one config object for Http, and one for Routing?

vojtajina commented

Yes, you are right. Sorry, I missed that your example had a config object
per token.

On Thu, Aug 7, 2014 at 3:06 PM, Alxandr notifications@github.com wrote:

But doesn't that mean you have 1 global (potentially HUGE) config object?
And not one config object for Http, and one for Routing?

—
Reply to this email directly or view it on GitHub
#56 (comment).

RGBboy commented

What happens in the instance where your config depends on the return value of another service? Or if you need a promise to resolve before you can get your config? This is handled nicely where configs are treated like all other dependencies.

With respect to merging, if issue #51 (to create a pattern for decorating a provider) was added your configs could be decorated by using the original config token. I would imagine that child injectors would respect the config associated with their parent so this would work:

class HttpConfig {
    constructor() {
        // default config here
        this.useJsonP = true;
        this.preventCache = false;
    }
}

@Inject(HttpConfig)
class Http {
    constructor(config) {
    }
}

// Decorate HttpConfig (not sure exactly how this would be written)
@Inject(HttpConfig)
@Provide(HttpConfig)
class HttpConfig1  {
    constructor(config) {
        Object.assign(this, config);
        this.useJsonP = false;
    }
}

@Inject(HttpConfig)
@Provide(HttpConfig)
class HttpConfig2  {
    constructor(config) {
        Object.assign(this, config);
        this.preventCache = true;
    }
}

var di1 = new Injector([HttpConfig1, Http]);
var di2 = di1.createChildInjector(HttpConfig2);  
// HttpConfig2 is still just a lookup but decorated so properties set 
// in HttpConfig1 are respected

This also solves the problem of deep vs shallow. You make the decision in the way you create your overriding config. In the example above it would be a deep merge but you could create your own config or decorate another config object to get the desired result.

I feel that adding a special config just adds more complexity to something that is already quite complicated. Treating them the same as other dependencies keeps it simple.

Alxandr commented

Yes, but you also go back to the (imho) icky convention-based way of doing config, where I have to either check some spec to see what the config name is, or just guess that it's the name of whatever I'm working with + config.

RGBboy commented

Surely you would have to check the spec if you want to know the properties that you can configure anyway.

Alxandr commented

Well, yeah. That's probably true. It just feels a bit like we're back in magic string land, but that's just my opinion.

Also, I think having config that depends on async stuff that is added to the injector is an anti-pattern. If your config depends on async stuff (like server resources), it should be resolved before the di is instansiated.

vojtajina commented

I think this convention can be decided by the user of DI, that is by framework such as Angular or a concrete app. DI should not be concerned about it.

It feels like we just need a multi-binding (multiple providers for the same token) or decorators or something like that, so that Angular can implement easy configuration on the top of it.

spralle commented

I'm curious about the ideas around multi-binding. Coming from the Java-world and been working with guice multi-binding is a very important part of DI. Is there any work in this direction in di.js? If not are there any ideas that I could work with and hopefully contribute code as this is an important area for me to create a pluggable architecture.