Skip to content

Commit

Permalink
Overriding setupController keeps default behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
tomhuda committed Jan 16, 2013
1 parent c317e9f commit 43354a9
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
8 changes: 6 additions & 2 deletions packages/ember-routing/lib/system/controller_for.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ Ember.generateController = function(container, controllerName, context) {
var controller;

if (context && Ember.isArray(context)) {
controller = Ember.ArrayController.extend({content: context});
controller = Ember.ArrayController.extend({
content: context
});
} else if (context) {
controller = Ember.ObjectController.extend({content: context});
controller = Ember.ObjectController.extend({
content: context
});
} else {
controller = Ember.Controller.extend();
}
Expand Down
10 changes: 5 additions & 5 deletions packages/ember-routing/lib/system/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ Ember.Route = Ember.Object.extend({

var controller = this.controllerFor(this.templateName, context);

if (controller) {
set(controller, 'model', context);
}

if (this.setupControllers) {
Ember.deprecate("Ember.Route.setupControllers is deprecated. Please use Ember.Route.setupController(controller, model) instead.");
this.setupControllers(controller, context);
Expand Down Expand Up @@ -208,11 +212,7 @@ Ember.Route = Ember.Object.extend({
@method setupController
*/
setupController: function(controller, model) {
if (controller) {
controller.set('content', model);
}
},
setupController: Ember.K,

/**
Returns the controller for a particular route.
Expand Down
29 changes: 29 additions & 0 deletions packages/ember/tests/routing/basic_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,35 @@ test("The Homepage with a `setupController` hook", function() {
equal(Ember.$('ul li', '#qunit-fixture').eq(2).text(), "Sunday: Noon to 6pm", "The template was rendered with the hours context");
});

test("The default controller's model is still set when overriding the setupController hook", function() {
Router.map(function() {
this.route("home", { path: "/" });
});

App.HomeRoute = Ember.Route.extend({
model: function() {
return {
isModel: true
};
},

setupController: function(controller) {
// no-op
// importantly, we are not calling this._super here
}
});

Ember.TEMPLATES.home = Ember.Handlebars.compile(
"<ul>{{#each entry in hours}}<li>{{entry}}</li>{{/each}}</ul>"
);

container.register('controller', 'home', Ember.Controller.extend());

bootApplication();

deepEqual(container.lookup('controller:home').get('model'), { isModel: true }, "model is still set on controller");
});

test("The Homepage with a `setupController` hook modifying other controllers", function() {
Router.map(function() {
this.route("home", { path: "/" });
Expand Down

2 comments on commit 43354a9

@ralfschimmel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enables me to strip some lines out of the new router, nice.

@wildchild
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setupController: function(controller, model) {
  var data = this.getDataFromAPISynchronously();
  controller.set('content', data);
}

Is my content going to be vanished?

Update: D'oh, it will set model before hook. Forget it.

Please sign in to comment.