Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented #172 with promises #257

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbektimirov
Copy link

App.start now returns a promise which depends on all application widgets promisses. Just return a promise() in widget 'initialize' method to handle async loading. Sample code is bellow.

// test widget
define(function() {
  return {
    initialize: function() {
      var dfd = this.sandbox.data.deferred();

      this.html('I am a test');

      setTimeout(function () {
        dfd.resolve();
      }, 3000);

      return dfd.promise();
    }
  };
});

// main app
require(['lib/aura'], function(Aura) {
  Aura({ debug: { enable: true } }).start({ widgets: 'body' }).then(function() {
    // this callback will be called after test widget 
    // deferred object will resolved (after 3000 ms)
    console.warn("Aura app started !");
  });
});

App.start now returns a promise which depends on all application
widgets promisses. Just return a promise() in widget 'initialize'
method to handle async loading.
@addyosmani
Copy link
Member

Thanks for this change, @mbektimirov. Would it be possible to also add some docs (addition to the readme) covering this change?

@sbellity
Copy link
Member

sbellity commented Jun 4, 2013

Thanks @mbektimirov ill try to review it today
An you also update the tests ? They seem to be failing on Travis

One comment though I went through the code quickly and my first thought is that we could probably do it in a more generic way by emitting events whenever all the widgets started via a Widget.startAll call have finished initialization...

@rspindel
Copy link

@sbellity +1 on using events over promises. See this good article debating their different strengths within asynchronous architecture and also my recent comment on using the Finite State Machine pattern on synchronizing events. http://www.joezimjs.com/javascript/javascript-asynchronous-architectures-events-vs-promises/

@addyosmani
Copy link
Member

my first thought is that we could probably do it in a more generic way by emitting events whenever all the widgets started via a Widget.startAll call have finished initialization

I agree with this. @mbektimirov just pinging you again in case you have time to update the tests. This would be hugely appreciated.

@mbektimirov
Copy link
Author

I remember about tests, still in vacation, sorry for delay. I'll fix this in about 3-4 days.

@addyosmani
Copy link
Member

ping @mbektimirov in case you have time :)

@addyosmani
Copy link
Member

@sbellity for our next release, do you feel this is important enough to write the tests ourselves and make it more generic then land? If not, we can certainly hold off on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants