jaredhanson / electrolyte

Elegant dependency injection for Node.js.
MIT License
564 stars 61 forks source link

Async creation of components #9

Open shaharke opened 10 years ago

shaharke commented 10 years ago

Is there any intention to support async creation of components? For example, a connection to db should not be injected into another component until it is actually established (something that can be confirmed only asynchronously).

Ideally, a component could return a promise and creation of dependent components should be conditioned on the fulfillment of that promise.

TomKaltz commented 10 years ago

+1

uetkaje commented 9 years ago

+1

leonfs commented 9 years ago

+1

StyleT commented 8 years ago

+1

ORESoftware commented 8 years ago

+1 I don't see how how DI is useful in node.js at all unless it does the work of procuring deps that require asynchronous procurement

ORESoftware commented 8 years ago

requirejs is the perfect example of useful IoC in js-land

ssljivic-vicert commented 8 years ago

+1

jaredhanson commented 8 years ago

Anyone want to implement this and submit a PR?

helmoski commented 8 years ago

I haven't been able to come up with a way to do this without making significant changes to electrolyte's api. The problem lies in trying to use the asynchronous results synchronously.

I first looked at updating FactorySpec's instantiate() method to use ES7 async/await syntax (courtesy of Nodent):

// Original
FactorySpec.prototype.instantiate = function() {
  debug('instantiate %s', this.id);
  return this._fn.apply(undefined, arguments);
}

// Updated with async/await
FactorySpec.prototype.instantiate = async function() {
  debug('instantiate %s', this.id);
  return await instantiateHelper(this);
}

function instantiateHelper(spec) {
  return new Promise((resolve, reject) => {
    Promise.resolve(spec._fn.apply(undefined, arguments)).then(obj => {
      resolve(obj);
    }).catch(e => {
      reject(e);
    });
  });
}

Originally, I thought that this would allow instantiate() to be called in the same way as before; however, I later discovered that the calling function must also be an async function which awaits the result of instantiate(). This requirement persists all the way up the call stack, so in order to get async/await to work, we would need to call the api's create() method like so:

var obj = await IoC.create('myDependency');

I next looked at using ES6 generators; however, this would require us to wrap all of our synchronous code in a generator.

The last thing I (skeptically) tried was using a while loop to block execution until the promise had been resolved because there were a few stackoverflow comments and a blog post that claimed it would work (I didn't notice that the blog post author admitted it wouldn't work in the comments until after I tried it):

// Example
function async() {
  return new Promise(resolve => {
    setTimeout(() => {
        resolve("DATA");
    }, 1000);
  });
}

function sync() {
  var result;
  var promise = async();
  promise.then(data => {
    result = data;
  });
  while(typeof result === 'undefined') { }
  return result;
}

var result = sync();
console.log(result);

As you would expect, this didn't work because the blocking while loop prevents the promise from resolving.

There may be a solution that someone smarter than me could come up with, but as far as I know, achieving the desired functionality without changing the api is not possible. I recommend updating the create() method to return a promise. This seems to be the typical approach taken by other IoC container modules, Even the dependency injection framework used by AngularJS 2.0 handles asynchronous dependencies this way.

TL;DR

I could not find a way to support asynchronous components without modifying electrolyte's api. I recommend updating the create() method to return a promise.

patrickhulce commented 8 years ago

quick WIP stab at async creation via a new createAsync method https://github.com/jaredhanson/electrolyte/pull/46

comments and feedback welcome

patrickhulce commented 8 years ago

@jaredhanson added examples and documentation to the PR and comfortable taking it out of WIP. would be interested to get your opinion on it. #46

happy 4th weekend folks!

goulashify commented 8 years ago

Ran into the same problem, created Wrappa, at some cases it might be usable, hope it spares some of the inconveniences!:)

Cheers, -Dan

slavafomin commented 7 years ago

Hello!

Thank you for this great module!

However, the asynchronous dependencies implementation has serious drawbacks in my opinion right now.

First of all, you have to specify that service factory is asynchronous in the service module itself. Then, you need to specify it in every module that depends on it. And you also need to use special createAsync method to retrieve the service manually. This creates a configuration which is spread around the application and when you decide to make a synchronous service factory asynchronous you will have to change find all places where it's defined and change it accordingly. This violates the DRY principle and makes the code non-maintainable.

The first solution would be to advice the end developers to always mark their services as asynchronous and to return Promise even if the service could be initialized synchronously. This will allow to switch to async model in the future without hassle. However, you will have to add boilerplate everywhere to mark all your services as async.

I would suggest a more radical approach: change the API of the container to always treat all services as asynchronous. That way you could remove this configuration hassle altogether. The end developer will be able to switch from sync to async model just by returning the Promise from the service factory instead of returning a simple value.

@jaredhanson Does it make sense?

Thanks!

patrickhulce commented 7 years ago

change the API of the container to always treat all services as asynchronous

I believe this was also @jaredhanson's proposal to my initial async implementation PR. I, for one, find it incredibly useful to enable both sync and async usage of electrolyte for situations in which one is restricted by an existing synchronous API. To do that, one of them must be picked by default, and it matched the historical API to go with sync.

I totally sympathize with the pain of annotating @async on every module though (along with @singleton!!), and I would love to see electrolyte options for setting attribute defaults on every module. There's been opposition to this in the past, but I think it's a real need.

shaharke commented 7 years ago

I, for one, find it incredibly useful to enable both sync and async usage of electrolyte for situations in which one is restricted by an existing synchronous API.

@patrickhulce can you give an example where there's a strong restriction to use sync API?

slavafomin commented 7 years ago

@patrickhulce Your service factory could be synchronous and return a simple value instead of Promise. It's a job of the container to detected the returned type and act accordingly. Actually, it's enough to just wrap the value return from service factory with Promise.resolve() in order to support both sync and async factories.