angular / di.js

Dependency Injection Framework for the future generations...
Other
818 stars 95 forks source link

How can I instantiate a class with injected arguments and custom arguments? #22

Open RGBboy opened 10 years ago

RGBboy commented 10 years ago
export class MyClass {

  @Inject(Service)
  constructor (service, customArgument) {
    this.service = service;
  }

  ...

}

My application has many instances of MyClass each instantiated with different customArgumnent. Is there a way to get DI to provide the injected arguments? I was thinking something similar to how in angular you can provide locals to the $injector.instantiate(expression, locals):

injector.instantiate(MyClass, { customArgument: 'customArgument' });
vojtajina commented 10 years ago

At this point you can do something like this:

class MyClass {
  constructor(service, customArgument) {
    // ...
  }
}

@Inject(Service)
export function instantiateMyClass(service) {
  return function(customArgument) {
    return new MyClass(service, customArgument);
  };
}

// Then, use the prebound factory (instantiateMyClass),
// rather than MyClass directly.
class SomeOther {
  @Inject(instantiateMyClass)
  constructor(instantiateMyClass) {
    for (var i = 0; i < 10; i++) {
      instantiateMyClass(i);
    }
  }
}

You could also ask for Injector and create a child scope with the custom arguments, but I don't like that. Everytime you ask for Injector, it smells bad, so I would like to cover at least most of the use cases without injecting Injector (at that point it is basically service locator). The code above is pretty verbose, I think we should support this as "prebound constructors". I discussed this more in https://docs.google.com/document/d/1LWxs4urzg-yQSzXOc_R8063Q14lemVqYcxd-Yp8BgZc/edit#bookmark=id.6ehf6thtt33v

RGBboy commented 10 years ago

Thanks @vojtajina. I just figured this out yesterday and came to the same conclusion.

vojtajina commented 10 years ago

Thanks for feedback, this is very valuable.

On Wed, Mar 5, 2014 at 11:54 AM, RGBboy notifications@github.com wrote:

Thanks @vojtajina https://github.com/vojtajina. I just figured this out yesterday and came to the same conclusion.

Reply to this email directly or view it on GitHubhttps://github.com/angular/di.js/issues/22#issuecomment-36785773 .

jeffbcross commented 10 years ago

+1 make this possible.

vojtajina commented 10 years ago

Guys, check out https://docs.google.com/document/d/1xzVDwz-hvkoaVPvHuFnm_FT7hoyvE1jSq2q3ffPB4BQ/edit#heading=h.r5y2l3pb7qvt

vojtajina commented 10 years ago

@RGBboy now, you can try this


class MyClass {
  @Inject(Service, 'some-token')
  constructor(service, custom) {}
}

class Foo {
  constructor(@InjectLazy(MyClass) createMyClass) {
    createMyClass('some-token', 'injected value'); 
    createMyClass('some-token', 'a different value');
  }
}
RGBboy commented 10 years ago

What if instead of defining tokens for the custom arguments the injector just concats what is defined in the Inject annotation with the values you pass to the function provided by the InjectLazy. E.g:

class MyClass {
  @Inject(Service) // note only Service is injected
  constructor(service, custom) {}
}

class Foo {
  constructor(@InjectLazy(MyClass) createMyClass) {
    createMyClass('injected value'); 
    createMyClass('a different value');
  }
}

I am just thinking that it could get quite confusing when you are trying to pass in multiple custom arguments. DI already has a bit of a learning curve and trying to keep its API as simple as possible would benefit everyone using it.

Would this be possible? Are there any downsides to this approach?

vojtajina commented 10 years ago

@RGBboy I'm not super happy with the current API either, but I don't know how to make it better ;-) Definitely something I'm open to a discussion...

I considered your proposal before. The reason why I didn't like it was that MyClass is saying what argument is being injected by the DI and what arguments are passed in by the consumer... I think MyClass should not worry about this. I think the consumer (Foo in your case) should decide. Also, MyClass can not be instantiated with just DI, it does require manually passed arguments.

I definitely want to keep the API as simple as possible.

joshball commented 10 years ago

I am not sure if this is just a ES5/6 syntax that I am missing, but I am trying to figure out how to instantiate an object with parameters, in ES5. What is the proper syntax for this:

var Car = function (engine, vin, color) { this.VIN = vin; this.color = color; this.engine = engine; }; di.annotate(Car, new di.Inject(Engine));

var Engine = function (vin, engineSize) { this.VIN = vin; this.engineSize = engineSize; this.state = 'stopped'; };

How do we create this? This would be traditional: var engine = new Engine('theVin', 'carEngineSize'); var car = new Car(engine, 'theVin', 'red');

I think I want this: var carOptions = { vin: 'theVin', color: 'red', engineSize: 'big' }; var car = injector.get(Car, carOptions);

// DI knows car needs an Engine and a VIN, gets Engine from DI and VIN from parameter. // DI knows that Engine needs a VIN and Size, and gets it from Car parameters // Not sure about the annotations. I see InjectLazy, but parameters aren't really things to be created (injected). // ??? di.annotate(Car, new di.Inject(Engine), new di.InjectLazy(String));

// I could get around this with an init method, but unfortunately in this case, I am wrapping a 3rd party library that takes a key parameter in the constructor (like the VIN in the car).

Thoughts?

vojtajina commented 10 years ago

You can use LazyInject:

di.annotate(Engine, new di.TransientScope);
di.annotate(Car, new di.Inject(Engine, 'vin', 'color'));
function Car(engine, vin, color) {
  // ...
}

di.annotate(Engine, new di.TransientScope);
di.annotate(Engine, new di.Inject('vin', 'engineSize'));
function Engine(vin, size) {
  // ...
}

di.annotate(run, new di.InjectLazy(Car));
function run(createCar) {
  var car1 = createCar(
    'vin', 'theVin', 
    'color', 'red',
    'engineSize', 'big'
  );
  var car2 = createCar(
    'vin', 'theVin2', 
    'color', 'blue',
    'engineSize', 'small'
  );
}

var injector = new di.Injector();
injector.get(run);

Or in your example, it would be shorter to do it manually:

function Engine(vin, size) {
  // ...  
}

function Car(engine, vin, color) {
  // ...
}

di.annotate(createCar, new di.Inject);
function createCar() {
  return function(vin, color, engineSize) {
    return new Car(new Engine(vin, engineSize), vin, color);
  }
}

di.annotate(run, new di.Inject(Car));
function run(createCar) {
  var car1 = createCar('theVin', 'red', 'big');
  var car2 = createCar('theVin2', 'blue', 'small');
}

var injector = new di.Injector();
injector.get(run);
Rush commented 10 years ago

Why can't we have code similar to what @joshball described? I also need ES5 support. The last example is very ugly, I was too looking to use di.js for relatively simple things but this is just scary. Looks like writing my custom tailored DI mechanism would yield nicer code ... I personally want DI to not only have better manageable code but also NICER code. Sorry for this rant but I had high hopes for your module.

vojtajina commented 10 years ago

You can make it nicer using some helper like this https://gist.github.com/vojtajina/9673132

@RushPL What nicer syntax do you suggest? I think using the annotations looks pretty good.

As for @joshball's suggestion, let's take this code...

var Car = function (engine, vin, color) {
  // ...
};
di.annotate(Car, new di.Inject(Engine));

var Engine = function (vin, engineSize) {
  // ...
};

How would you instantiate Car?

di.annotate(main, new Inject(Car));
function main(car) {
  // ... what here? car can't be instance of Car,
  // you need provide missing dependencies (Car -> vin, Car -> color, Engine -> vin, Engine ->engineSize)
  // HOW? How do you specify what is Engine deps and what Car deps?
}

Any suggestions?

Rush commented 10 years ago

note: I am no expert in DI...

injector.get(main, {
  vin: fetchVin(),
  engineSize: engineSize()
});

or

injector.onMissing(function(name, cb) {
  cb(null, customDeps[name]);
});
injector.get(main);

In my opinion the injector should not be fanatical about purity and should instead be flexible to accomodate wild use cases but of course Angular.JS has its own needs so I wouldn't know. I know that I am looking for good DI for my own project in ES5 and this isn't it (yet).

vojtajina commented 10 years ago

The first example assumes using strings as tokens. How would you make it work with a non string tokens? (This is exactly how InjectLazy works, just the syntax is not as nice because of non-string tokens)

The second example would work with non-string tokens fine. I've been thinking about having a "missing_dependency" callback for a while. I'm still not sold on it, but maybe it's a good idea, I will try it. That said, I don't like using "missing dependency" callback for stuff like this example. I mean, this is no missing dependency, right? Missing dependency sounds like something went wrong. It's like using exceptions for control flow...

I do wanna solve this problem, I'm just not sure how. That's why all this feedback is very welcomed.

Alxandr commented 10 years ago

The first example can be argued that isn't di at all. It's just allowing to pass arguments out of order (or by name, if you like). If you wanted to provided dependent dependencies with a system like that (which as I argued, isn't even di) you end up with the following:

var logger = injector.get(logger, {
  write: console.log.bind(console)
});
injector.get(main, {
   vin: injector.get(fetchVin, {
    logger: logger
  }),
  engine: injector.get(engine, {
    logger: logger
  })
});

and this is a really simple dependency-graph. In languages that support strong typing and reflection (metaprogramming), dependency-injection is often achieved by use of the built-in type-system in the language. For instance, I could rewrite the code above to the following pseudo-code:

interface IConsole { /* methods */ }
interface ILogger { /* methods */ }
interface IVin { /* methods */ }
interface IEngine { /* methods */ }

class Logger : ILogger {
  public Logger(IConsole console) {}
  /* methods */
}

class Vin : IVin {
  public Vin(ILogger logger) {}
  /* methods */
}

class Engine : IEngine {
  public Engine(ILogger logger) {}
  /* methods */
}

class Console : IConsole {
  public Console() {}
  /* methods */
}

class NullLogger : ILogger {
  public NullLogger() {}
  /* methods */
}

void Main(IVin vin, IEngine engine) {

}

and then somewhere allong the line say di.Execute(Main). The point of interest here is the jobs the di has to perform to do such a call. First it has to figure out that main needs an IVin and an IEngine. Then it has to figure out what to supply for both of those arguments. For now let's assume we've already configured the di to use Engine as IEngine and Vin as IVin etc. So the di figures out that it can provide an engine and a vin to the main. Than it has to see that both Engine and Vin requires a ILogger, which can be fulfilled by a Logger, which in turn requires a IConsole etc. And that's just the first part of the problem, namely discovery.

The next part is where it really get's tricky. As you're doing debugging in this application, you've figured out that there's a bug in the engine, while the vin works just fine, how ever, just running generates too much log-output to find out where things go wrong, so you would like to provide a NullLogger to the Vin so that you only get log output from the engine without changing any of the code above.

So let's take a look at how one might setup a system like this in a typed language, and I'll draw the parallels to javascript afterwords.

// create new di
var di = new DependencyInjector();
di.Register<IConsole, Console>();
di.Register<ILogger, Logger>();
di.Register<IVin, Vin>().InContext(ctx => {
  // We want the Vin to get the NullLogger
  // Note: this could have been done the other way arround
  // and the NullLogger could have been default, in which
  // case we'd just provide the Logger to Engine
  ctx.Register<ILogger, NullLogger>();
});
di.Register<IEngine, Engine>();

// finally, start off the whole thing
di.Execute(Main);

Now, if I wanted to swap out the default console with something else entirely, that'd be easy. I'd just go to my di config, and be done. So there's two kinds of information given to the di here. First there's the di config. That's one easy to translate to javascript, you can simply do di.register(IConsole, Console) instead of using generic methods. You loose the typing obviously, but as javascript doesn't have typing, you never had that in the first case. There are multiple ways of handling the scenario with the Vin context-wise, and I'm not sure how di.js does this today, but beeing able to register this kind of stuff in a central place is important in a di imho.

The second part of the information given to the di is the type-information. When I created the class Logger I created a main-method that looks like this:

public Logger(IConsole console)

This tells the dependency-injector that the Logger needs an IConsole of some sort. Now, obviously since javascript doesn't have typing, we can't use the same technique. So therefore the di.js team came up with the following syntax:

@Inject(IConsole)
class Logger extends ILogger {
  constructor(console) {}
  /* methods */
}

Now, as can be seen, this obviously isn't ES5, but it can be translated quite nicely into ES5 code like this:

function Logger() {}
Logger.prototype = Object.create(ILogger.prototype); // subclass
/* add methods to the prototype */
di.annotate(Logger, new Inject(IConsole));

On a side note, I have to go a little off-topic to ask the di.js team a question. Given that traceur can enable types in javascript (should one wish for it), can the di as of today use the type-information? Cause if I could simply write the code above as

class Logger extends ILogger {
  constructor(console: IConsole) { }
  /* methods */
}

that would be really awesome. I'd also like to know how one would resolve the NullLogger sample I provided in todays di.

vojtajina commented 10 years ago

@Alxandr I agree with your explanation.

DI.js understands Traceur types (probably better to call it "contracts", it's part of ES6++; we've been using it on angular v2, see https://github.com/angular/templating/blob/master/src/view.js#L18 - it gets injected based on the type annotations).

Regards your example of NullLoger, there is no syntax for this right now. If I needed overide a single dependency for debugging, I would just go into the Vin constructor and instantiate NullLoger manually. It might be possible with private modules which is something I've been thinking about but it's not implemented and right now I have other things I want to solve first, such as this issue (partial injection/prebound-constructors) or forcing new instances.

vojtajina commented 10 years ago

@RushPL @joshball Could you give me a real example (instead of Car/Engine)? I'm looking for a good real example of passing some dependency through (eg. like Car -> Engine -> VIN in your example). I assume you have a real example in your app. Can you share it? I think it might help us to find a better solution to this problem....

How would the Car/Engine example fit into a bigger app picture? (I kind of expect the real example will answer this question)

@RGBboy or maybe your MyClass? Can you share more details about the app?

OliverJAsh commented 9 years ago

@vojtajina Perhaps I can provide a real world example.

I have an ApiClient class that needs a HttpAdapter injected into it. The ApiClient is to be published without a HttpAdapter, leaving the consumer to "provide" one. Here is what I tried:

/* jshint esnext: true */
import * as di from 'di/src';

class HttpAdapter {
    constructor() {
        console.log('HttpAdapter constructor');
    }
}

class ApiClient {
    constructor(httpAdapter) {
        console.log('ApiClient constructor httpAdapter: %s', httpAdapter);
    }
}
// TODO: Fix ES6 class annotations
// As per: https://github.com/angular/di.js/issues/80
di.annotate(ApiClient, new di.Inject('HttpAdapter'));

di.annotate(run, new di.Inject(HttpAdapter));
di.annotate(run, new di.InjectLazy(ApiClient));
function run(httpAdapter, createApiClient) {
    createApiClient('HttpAdapter', httpAdapter);
}

var injector = new di.Injector();

injector.get(run);

As per: https://gist.github.com/1e8bacc182ecc9393072

Unfortunately this doesn't work:

/Users/Oliver/Development/sbscribe/test.js:22
    createApiClient('HttpAdapter', httpAdapter);
    ^
TypeError: Error during instantiation of run!
ORIGINAL ERROR: undefined is not a function
    at run (/Users/Oliver/Development/sbscribe/test.js:22:5)
    at FactoryProvider.create (/Users/Oliver/Development/sbscribe/node_modules/di/src/providers.js:125:24)
    at Injector.get (/Users/Oliver/Development/sbscribe/node_modules/di/src/injector.js:246:25)
    at Object.<anonymous> (/Users/Oliver/Development/sbscribe/test.js:27:8)

I expected that I would just be able to define a provider for the injectable, like so:

/* jshint esnext: true */
import * as di from 'di/src';

class HttpAdapter {
    constructor() {
        console.log('HttpAdapter constructor');
    }
}
di.annotate(HttpAdapter, new di.Provide('HttpAdapter'));

class ApiClient {
    constructor(httpAdapter) {
        console.log('ApiClient constructor httpAdapter: %s', httpAdapter);
    }
}
// TODO: Fix ES6 class annotations
// As per: https://github.com/angular/di.js/issues/80
di.annotate(ApiClient, new di.Inject('HttpAdapter'));

var injector = new di.Injector();

var apiClient = injector.get(ApiClient);

Of course this doesn't work.

Not sure if my ideas here add up, but hopefully this provides you will a real world example.

nateabele commented 9 years ago

Maybe this has already been proposed, but what about allowing get() to accept an array of locals that match injected parameters by constructor? For example:


class Bar {
  // ...
}

@TransientScope
class Foo {
  constructor(@Inject(Bar) bar) {
    // ...
  }
}

var regularFoo = injector.get(Foo);
var customFoo = injector.get(Foo, [new Bar({ custom: true })]); // <-- "locals"

Since Foo.parameters[0].tokens[0] === locals[0].constructor... match :+1:. Hopefully that makes sense.

Naturally, this would be problematic if you had more than one of the same constructor in either list, but that sort of thing is probably Wrong™ anyway.

Granted, directly accessing the injector is often a bad "smell", but at the system level, managing object construction is a complicated data flow problem, and without an API that provides greater control, it is kind of a necessary evil. Sorry I don't have any better ideas. :-/

Joebeazelman commented 5 years ago

Why not copy what .NET has? .NET has a long history of dependency injection with numerous official and third party libraries battling it out. With the new .NET Core, DI is now built-in and just works with minimal configuration. .NET has facilities deep in its CLR to support such as reflection where an object can be queried for its metadata describing an object's type. Typescript inherently supports this as well.

Here's a comprehensive list of open source DI libraries for .NET and how they address issues such as injecting constructor parameters.

davija commented 4 years ago

How about something like a @Ignore() decorator that tells DI to not try and inject that attribute? The use case I'm dealing with has a base class that MUST be a component (has @Input() and @Output()) but only takes in arguments from derived components.