Using Injector for non-singleton classes #4

Closed
joliss opened this Issue Feb 1, 2014 · 10 comments

Projects

None yet

4 participants

@joliss
Contributor
joliss commented Feb 1, 2014

Hey Vojta, thanks for di.js!

I'm wondering if there's a good way to use the Injector to create non-singleton classes. For instance, say we have

var amdTranspiler = new ES6Transpiler({
  target: 'amd',
  loader: 'myloader.js',
  ...
});

var nodeTranspiler = new ES6Transpiler({ ... });

And we'd like for the ES6Transpiler to take some injections. It seems that injector.get(ES6Transpiler) is fundamentally the wrong way, since it's used for singletons.

We could just pass the injector as the first constructor argument,

var injector = new Injector([]);
var amdTranspiler = new ES6Transpiler(injector, { ... });

But it seems that this makes the implementation inside of ES6Transpiler rather ugly, since we don't get to use annotations, so we'd have to do injector.get(Foo) inside of the ES6Transpiler constructor.

I'm thinking we could add an injector.create method for this, which works like .get, but doesn't cache - but I haven't thought this through at all.

Do you have any thoughts on this? Are there existing patterns? Thanks :)

@joliss
Contributor
joliss commented Feb 3, 2014

I have edited & re-written the question completely to make more sense (previous issue title was "Additional constructor args")

@vojtajina
Contributor

In general, life scope of objects is managed through child injectors.

When using the DI with AngularJS, Angular will probably take care of managing these child injectors, based on declarative annotations. For instance, let's say you wanna create a new instance of SomeCache object per application route.

@RouteScope
class SomeCache {}

// inside Angular:
// Angular creates new injector (per route) and forces new instances of all objects with some annotation (RouteScope in this case)
var currentRouteInjector = injector.createChild(additionalModulesToLoad, forceNewInstanceForAllProvidersWithAnnotation(RouteScope));

// everything inside this route will be instantiate with this child injector
var newCachePerRoute = childInjector.get(SomeCache);

So as you can see, this also enables lazy loading of code - when changing a route, you can load additional code.

Note that having a child injector "per route" is just an example. Your app can have many "scopes".

Now, for your example I think we might have some kind of "AlwaysNewInstance" annotation. This is all not entirely clear yet - there are many other ideas and I think it will get polished soon, once we start building real stuff with it.

You might also check out the out-dated design doc.

@joliss
Contributor
joliss commented Feb 16, 2014

Hm, child injectors didn't really solve my use case, because they still assume that you'll have one instance of the class per child injector. But what I'd want is possibly a bunch of instances, each ininitialized with different parameters to the constructor.

I'm not sure if there's a good solution at all. Let's close this issue for now.

@joliss joliss closed this Feb 16, 2014
@MikeMcElroy
Contributor

I'm really not clear on why it would be beneficial for the DI system to
handle this... if you want new objects, import the module which exports
your desired object and create a new object within your code, or have a
factory class that you can interact with to create instances (similar to
how the factory classes can return prototype constructors you can call with
new in AngularJS 1.x). Having the DI system create new instances leaves
you open to memory leaks or added complexity. To my way of thinking, the
DI system is more on the side of service discovery and supplying
dependencies rather than object allocation per se .

But to be fair, I think the DI/IoC container I worked with in .NET MVC
actually created new instances of the dependencies each time it was called
(I'm about 51% confident in that). The singleton approach is really
powerful (obviously), but perhaps if you had a specific class you wanted
individual instances to be created on within the same injector, perhaps you
could create a SingletonAnnotation which would signal whether or not the
dependency should be instantiated anew. The default for this annotation
would be true, so it would be non-breaking with the current way of Angular
DI. Does that sound appropriate, @vojtajina?

@joliss
Contributor
joliss commented Feb 16, 2014

Both using a factory class and having a SingletonAnnotation sound reasonable.

I'm not using di.js at the moment (as I managed to eliminate all the dependencies I was injecting), but next time someone has a real-world use case for this it might be worth revisiting.

@MikeMcElroy
Contributor

Thinking about this some more... there's a pattern already existing called Multiton which I think could be worked into what we're talking about here. In simple terms, the Multiton pattern maintains a cache of instances which are requested by outside entities by key, very similarly to how the current DI system maintains a cache of already-instantiated providers. It seems like a relatively simple process to create a @Multiton annotation which directs the DI system to save the instances in this way.

I've been thinking of how some of the examples might be modified to show a valid use case for this sort of thing, and it's been a bit difficult because I've had to play Devils Advocate on it a lot. I still am not entirely convinced it's a good idea. Will continue to muse on it.

@vojtajina
Contributor

I'm probably gonna add @TransientScope annotation, which will instruct the DI to always create a new instance. This might also enable stuff discussed in angular/diary.js#2.

Another related idea is "pre-bound classes". You could basically ask for a class which would get first x arguments injected and the rest would be passed by you, when calling it with new operator. I tried to describe it a little bit more in https://docs.google.com/document/d/1LWxs4urzg-yQSzXOc_R8063Q14lemVqYcxd-Yp8BgZc/edit#bookmark=id.6ehf6thtt33v.

@pkozlowski-opensource

@vojtajina would this new annotation be placed in addition to @Provide or instead of it? Sth like:

@TransientScope
@Provide(NotASingleton)
class NotASingleton {
}

?

@MikeMcElroy
Contributor

+1 for being able to affix multiple annotations, it just means care needs to be taken to make the interactions between the annotations sensible, which is added complexity for us in the design stages. Can Traceur support multiple annotations like this?

@vojtajina
Contributor

Yep, it would be another annotation. Traceur already supports that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment