Permalink
Browse files

Improve application readiness framework

- Allow other reasons for deferring readiness
  other than DOMContentLoaded
- Defer app.startRouting until all readiness
  deferrals are complete
- Start app with a single readiness deferral,
  which is advanced by a call to `initialize`.
- If you don't call initialize, you're gonna have
  a bad time.

There are two reasons for this change:

- Allow Ember Data to defer routing until the
  adapter has initialized. For example, an
  IndexedDB adapter needs to establish a
  connection before any queries can be performed.
- Ensure consistent behavior regardless of
  whether DOM is already ready or not when the
  application is loaded. Ensure that readiness
  hooks only run a single time.
  • Loading branch information...
tomhuda
tomhuda committed Aug 17, 2012
1 parent cb0a649 commit ba3e74e02d160fa870181a851c9e897fc66e4b6c
@@ -60,6 +60,8 @@ Ember.Application = Ember.Namespace.extend(
/** @private */
init: function() {
if (!this.$) { this.$ = Ember.$; }
var eventDispatcher,
rootElement = get(this, 'rootElement');
this._super();
@@ -70,14 +72,32 @@ Ember.Application = Ember.Namespace.extend(
set(this, 'eventDispatcher', eventDispatcher);
// jQuery 1.7 doesn't call the ready callback if already ready
if (Ember.$.isReady) {
// Start off the number of deferrals at 1. This will be
// decremented by the Application's own `initialize` method.
this._readinessDeferrals = 1;
this.waitForDOMContentLoaded();
},
waitForDOMContentLoaded: function() {
this.deferReadiness();
var self = this;
this.$().ready(function() {
self.advanceReadiness();
});
},
deferReadiness: function() {
Ember.assert("You cannot defer readiness since the `ready()` hook has already been called.", this._readinessDeferrals > 0);
this._readinessDeferrals++;
},
advanceReadiness: function() {
this._readinessDeferrals--;
if (this._readinessDeferrals === 0) {
Ember.run.once(this, this.didBecomeReady);
} else {
var self = this;
Ember.$(document).ready(function() {
Ember.run.once(self, self.didBecomeReady);
});
}
},
@@ -131,19 +151,28 @@ Ember.Application = Ember.Namespace.extend(
});
});
if (router && router instanceof Ember.Router) {
this.startRouting(router);
}
// At this point, any injections or load hooks that would have wanted
// to defer readiness have fired.
this.advanceReadiness();
return this;
},
/** @private */
didBecomeReady: function() {
var eventDispatcher = get(this, 'eventDispatcher'),
customEvents = get(this, 'customEvents');
customEvents = get(this, 'customEvents'),
router;
eventDispatcher.setup(customEvents);
this.ready();
router = get(this, 'router');
if (router && router instanceof Ember.Router) {
this.startRouting(router);
}
},
/**
@@ -14,11 +14,14 @@ module("Ember.Application", {
Ember.$("#qunit-fixture").html("<div id='one'><div id='one-child'>HI</div></div><div id='two'>HI</div>");
Ember.run(function() {
application = Ember.Application.create({ rootElement: '#one' });
application.initialize();
});
},
teardown: function() {
Ember.run(function(){ application.destroy(); });
if (application) {
Ember.run(function(){ application.destroy(); });
}
}
});
@@ -36,23 +39,23 @@ test("you can make a new application in a non-overlapping element", function() {
test("you cannot make a new application that is a parent of an existing application", function() {
raises(function() {
Ember.run(function() {
Ember.Application.create({ rootElement: '#qunit-fixture' });
Ember.Application.create({ rootElement: '#qunit-fixture' }).initialize();
});
}, Error);
});
test("you cannot make a new application that is a descendent of an existing application", function() {
raises(function() {
Ember.run(function() {
Ember.Application.create({ rootElement: '#one-child' });
Ember.Application.create({ rootElement: '#one-child' }).initialize();
});
}, Error);
});
test("you cannot make a new application that is a duplicate of an existing application", function() {
raises(function() {
Ember.run(function() {
Ember.Application.create({ rootElement: '#one' });
Ember.Application.create({ rootElement: '#one' }).initialize();
});
}, Error);
});
@@ -64,11 +67,11 @@ test("you cannot make two default applications without a rootElement error", fun
});
Ember.run(function() {
application = Ember.Application.create();
application = Ember.Application.create().initialize();
});
raises(function() {
Ember.run(function() {
Ember.Application.create();
Ember.Application.create().initialize();
});
}, Error);
});
@@ -106,7 +109,7 @@ test("initialize controllers into a state manager", function() {
var stateManager = Ember.Object.create();
app.initialize(stateManager);
Ember.run(function() { app.initialize(stateManager); });
ok(get(stateManager, 'fooController') instanceof app.FooController, "fooController was assigned");
ok(get(stateManager, 'barController') instanceof app.BarController, "barController was assigned");
@@ -147,7 +150,7 @@ test('initialized application go to initial route', function() {
app.ApplicationController = Ember.Controller.extend();
app.initialize(app.stateManager);
Ember.run(function() { app.initialize(app.stateManager); });
});
equal(app.get('router.currentState.path'), 'root.index', "The router moved the state into the right place");
@@ -285,12 +288,13 @@ test("ApplicationView is inserted into the page", function() {
});
test("ApplicationView and ApplicationController are assumed to exist in all Routers", function() {
Ember.run(function() {
app = Ember.Application.create({
rootElement: '#qunit-fixture'
});
});
Ember.run(function() {
app.OneView = Ember.View.extend({
template: function() { return "Hello!"; }
});
@@ -305,11 +309,10 @@ test("ApplicationView and ApplicationController are assumed to exist in all Rout
})
})
});
raises(function(){ app.initialize(); }, Error);
});
raises(function(){ Ember.run(function() { app.initialize(); }); }, Error);
});
test("ControllerObject class can be initialized with target, controllers and view properties", function() {
@@ -324,7 +327,7 @@ test("ControllerObject class can be initialized with target, controllers and vie
stateManager = Ember.StateManager.create();
app.initialize(stateManager);
Ember.run(function() { app.initialize(stateManager); });
stateManager.get('postController').set('view', Ember.View.create());
});
@@ -0,0 +1,134 @@
var jQuery, Application, application;
var readyWasCalled, domReady, readyCallback;
// We are using a small mock of jQuery because jQuery is third-party code with
// very well-defined semantics, and we want to confirm that a jQuery stub run
// in a more minimal server environment that implements this behavior will be
// sufficient for Ember's requirements.
module("Application readiness", {
setup: function() {
readyWasCalled = 0;
var jQueryInstance = {
ready: function(callback) {
readyCallback = callback;
if (jQuery.isReady) {
domReady();
}
}
};
jQuery = function() {
return jQueryInstance;
};
jQuery.isReady = false;
var domReadyCalled = 0;
domReady = function() {
if (domReadyCalled !== 0) { return; }
domReadyCalled++;
readyCallback();
};
Application = Ember.Application.extend({
$: jQuery,
ready: function() {
readyWasCalled++;
}
});
},
teardown: function() {
if (application) {
Ember.run(function() { application.destroy(); });
}
}
});
// These tests are confirming that if the callbacks passed into jQuery's ready hook is called
// synchronously during the application's initialization, we get the same behavior as if
// it was triggered after initialization.
test("Ember.Application's ready event is called right away if jQuery is already ready", function() {
jQuery.isReady = true;
Ember.run(function() {
application = Application.create().initialize();
});
equal(readyWasCalled, 1, "ready was called");
Ember.run(function() {
domReady();
});
equal(readyWasCalled, 1, "application's ready was not called again");
});
test("Ember.Application's ready event is called after the document becomes ready", function() {
Ember.run(function() {
application = Application.create().initialize();
});
equal(readyWasCalled, 0, "ready wasn't called yet");
Ember.run(function() {
domReady();
});
equal(readyWasCalled, 1, "ready was called now that DOM is ready");
});
test("Ember.Application's ready event can be deferred by other components", function() {
Ember.run(function() {
application = Application.create();
});
application.deferReadiness();
Ember.run(function() {
application.initialize();
});
equal(readyWasCalled, 0, "ready wasn't called yet");
Ember.run(function() {
domReady();
});
equal(readyWasCalled, 0, "ready wasn't called yet");
Ember.run(function() {
application.advanceReadiness();
});
equal(readyWasCalled, 1, "ready was called now all readiness deferrals are advanced");
});
test("Ember.Application's ready event can be deferred by other components", function() {
jQuery.isReady = true;
Ember.run(function() {
application = Application.create();
});
application.deferReadiness();
Ember.run(function() {
application.initialize();
});
equal(readyWasCalled, 0, "ready wasn't called yet");
Ember.run(function() {
application.advanceReadiness();
});
equal(readyWasCalled, 1, "ready was called now all readiness deferrals are advanced");
raises(function() {
application.deferReadiness();
}, Error);
});

7 comments on commit ba3e74e

@sly7-7

This comment has been minimized.

Show comment
Hide comment
@sly7-7

sly7-7 Aug 22, 2012

Contributor

@wycats @tomdale,

After I've answered the issue #1302, I was thinking if the job done in initialize() could/should be done in the init() method of the application. I don't know if it does make sense with Ember, or in Javascript (and more generally in web application), but for me, in oop, it is the role of the constructor to initialize an object (in this case the application). http://en.wikipedia.org/wiki/Constructor_(object-oriented_programming)

Basically, it would be more usual for me to declare the application with:

App = Ember.Application.extend({
   Router: Ember.Router.extend({...}),
   ApplicationView: Ember.View.extend({...}),
   ApplicationController: Ember.Controller.extend({..}),
   Store: DS.Store.extend({...}),
   // and all other declarations
});

and when calling App.create(), all the initialization and injections work are done, so the App would be ready.
I think this would not change anything from a user point of view, but from an OOP point of view, in my opinion it seems better.

What do you think about this ? Am I totally off topic ? Or perhaps am I wrong about this concept?

Contributor

sly7-7 replied Aug 22, 2012

@wycats @tomdale,

After I've answered the issue #1302, I was thinking if the job done in initialize() could/should be done in the init() method of the application. I don't know if it does make sense with Ember, or in Javascript (and more generally in web application), but for me, in oop, it is the role of the constructor to initialize an object (in this case the application). http://en.wikipedia.org/wiki/Constructor_(object-oriented_programming)

Basically, it would be more usual for me to declare the application with:

App = Ember.Application.extend({
   Router: Ember.Router.extend({...}),
   ApplicationView: Ember.View.extend({...}),
   ApplicationController: Ember.Controller.extend({..}),
   Store: DS.Store.extend({...}),
   // and all other declarations
});

and when calling App.create(), all the initialization and injections work are done, so the App would be ready.
I think this would not change anything from a user point of view, but from an OOP point of view, in my opinion it seems better.

What do you think about this ? Am I totally off topic ? Or perhaps am I wrong about this concept?

@michaelbaudino

This comment has been minimized.

Show comment
Hide comment
@michaelbaudino

michaelbaudino Aug 22, 2012

Being a former C++ programmer, I totally agree that conceptually, the init code should be located in the C-tor, having another initialize() method kind of looks like a code design hack.
The next question might be : do one needs to call initialize() after routing or injections ?
Maybe if you want to add a controller or a route dynamically, I guess.

michaelbaudino replied Aug 22, 2012

Being a former C++ programmer, I totally agree that conceptually, the init code should be located in the C-tor, having another initialize() method kind of looks like a code design hack.
The next question might be : do one needs to call initialize() after routing or injections ?
Maybe if you want to add a controller or a route dynamically, I guess.

@mattkime

This comment has been minimized.

Show comment
Hide comment
@mattkime

mattkime Aug 22, 2012

I've found the separate initialize fn to be useful since there is a lot of setup before it can be called. (pretty much the structure of the whole app)

We could configure the object passed to Ember.Application.create in the same manner.

It would be nice to do The Right Thing but I think this is a pretty minor point overall.

mattkime replied Aug 22, 2012

I've found the separate initialize fn to be useful since there is a lot of setup before it can be called. (pretty much the structure of the whole app)

We could configure the object passed to Ember.Application.create in the same manner.

It would be nice to do The Right Thing but I think this is a pretty minor point overall.

@trek

This comment has been minimized.

Show comment
Hide comment
@trek

trek Aug 22, 2012

Member

A two step object setup/object use phase is pretty common in OOP. Just search github for methods with names like start, connect, or perform and you'll see it all over.

Member

trek replied Aug 22, 2012

A two step object setup/object use phase is pretty common in OOP. Just search github for methods with names like start, connect, or perform and you'll see it all over.

@sly7-7

This comment has been minimized.

Show comment
Hide comment
@sly7-7

sly7-7 Aug 22, 2012

Contributor

I know this is common, but for me this is quite a bad smell. But anyway, I probably do that sometimes in my code, and my comment was only an thought, probably a bad one.

Contributor

sly7-7 replied Aug 22, 2012

I know this is common, but for me this is quite a bad smell. But anyway, I probably do that sometimes in my code, and my comment was only an thought, probably a bad one.

@trek

This comment has been minimized.

Show comment
Hide comment
@trek

trek Aug 22, 2012

Member
  Ember.boot = function(application, router){
    application.initialize(router)
  }

  Ember.boot(MyApp)

Boom. OOP. :shipit:

Member

trek replied Aug 22, 2012

  Ember.boot = function(application, router){
    application.initialize(router)
  }

  Ember.boot(MyApp)

Boom. OOP. :shipit:

@sly7-7

This comment has been minimized.

Show comment
Hide comment
@sly7-7

sly7-7 Aug 22, 2012

Contributor

I'm confused, I don't understand... It should be a troll or joke related to https://github.com/ksturner/ShipItSquirrel.
Anyway, with my original comment, I only wanted to know if it would be possible to not annoying the user with initialize(). It seems not.

Contributor

sly7-7 replied Aug 22, 2012

I'm confused, I don't understand... It should be a troll or joke related to https://github.com/ksturner/ShipItSquirrel.
Anyway, with my original comment, I only wanted to know if it would be possible to not annoying the user with initialize(). It seems not.

Please sign in to comment.