Skip to content

Commit

Permalink
App.container was not meant to be a public API
Browse files Browse the repository at this point in the history
It appears that people are starting to use
App.container.lookup as the public API for
globally getting other controllers outside of the
router or other controllers.

This was not intended to be a public API.

Instead, you should trigger events on a route
(or a parent route), which have access to
`this.controllerFor`.

Example:

    App.ApplicationRoute.extend({
      events: {
        search: function(term) {
          this.controllerFor('search')
            .set('term', term);
        }
      }
    });

    App.SearchField = Ember.TextField.extend({
      insertNewline: function() {
        var term = this.get('value'),
            controller = this.get('controller');

        controller.send('search', term);
        this.set('value', '');
      }
    });

Events sent to the controller will bubble up to
the current route, and then up the route hierarchy
  • Loading branch information
wycats committed Jan 5, 2013
1 parent 8fec737 commit 5becdc4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 11 deletions.
19 changes: 12 additions & 7 deletions packages/ember-application/lib/system/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ var Application = Ember.Application = Ember.Namespace.extend(

init: function() {
if (!this.$) { this.$ = Ember.$; }
this.container = this.buildContainer();
this.__container__ = this.buildContainer();
this.Router = this.Router || this.defaultRouter();

this._super();
Expand All @@ -285,7 +285,7 @@ var Application = Ember.Application = Ember.Namespace.extend(
@return {Ember.Container} the configured container
*/
buildContainer: function() {
var container = this.container = Application.buildContainer(this);
var container = this.__container__ = Application.buildContainer(this);

return container;
},
Expand Down Expand Up @@ -402,6 +402,11 @@ var Application = Ember.Application = Ember.Namespace.extend(
}
},

register: function() {
var container = this.__container__;
return container.register.apply(container, arguments);
},

/**
@private
Expand All @@ -419,7 +424,7 @@ var Application = Ember.Application = Ember.Namespace.extend(
this.isInitialized = true;

// At this point, the App.Router must already be assigned
this.container.register('router', 'main', this.Router);
this.__container__.register('router', 'main', this.Router);

// Run any injections and run the application load hook. These hooks may
// choose to defer readiness. For example, an authentication hook might want
Expand All @@ -440,9 +445,9 @@ var Application = Ember.Application = Ember.Namespace.extend(
@method runInitializers
*/
runInitializers: function() {
var router = this.container.lookup('router:main'),
var router = this.__container__.lookup('router:main'),
initializers = get(this.constructor, 'initializers'),
container = this.container,
container = this.__container__,
graph = new Ember.DAG(),
namespace = this,
properties, i, initializer;
Expand Down Expand Up @@ -517,7 +522,7 @@ var Application = Ember.Application = Ember.Namespace.extend(
@property router {Ember.Router}
*/
startRouting: function() {
var router = this.container.lookup('router:main');
var router = this.__container__.lookup('router:main');
if (!router) { return; }

router.startRouting();
Expand All @@ -537,7 +542,7 @@ var Application = Ember.Application = Ember.Namespace.extend(
var eventDispatcher = get(this, 'eventDispatcher');
if (eventDispatcher) { eventDispatcher.destroy(); }

this.container.destroy();
this.__container__.destroy();
},

initializer: function(options) {
Expand Down
10 changes: 6 additions & 4 deletions packages/ember-application/tests/system/application_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ test('initialized application go to initial route', function() {
match("/").to("index");
});

app.container.register('template', 'application',
app.register('template', 'application',
Ember.Handlebars.compile("{{outlet}}")
);

Expand Down Expand Up @@ -144,7 +144,9 @@ test("initialize application via initialize call", function() {
});
});

var router = app.container.lookup('router:main');
// This is not a public way to access the container; we just
// need to make some assertions about the created router
var router = app.__container__.lookup('router:main');
equal(router instanceof Ember.Router, true, "Router was set from initialize call");
equal(router.location instanceof Ember.NoneLocation, true, "Location was set from location implementation name");
});
Expand All @@ -165,12 +167,12 @@ test("initialize application with stateManager via initialize call from Router c
match("/").to("index");
});

app.container.register('template', 'application', function() {
app.register('template', 'application', function() {
return "<h1>Hello!</h1>";
});
});

var router = app.container.lookup('router:main');
var router = app.__container__.lookup('router:main');
equal(router instanceof Ember.Router, true, "Router was set from initialize call");
equal(Ember.$("#qunit-fixture h1").text(), "Hello!");
});
Expand Down

11 comments on commit 5becdc4

@tomdale
Copy link
Member

@tomdale tomdale commented on 5becdc4 Jan 5, 2013

Choose a reason for hiding this comment

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

Every time we catch someone recommending calling App.__container__.lookup in IRC, we will add another pair of underscores.

@mikegrassotti
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@stefanpenner
Copy link
Member

Choose a reason for hiding this comment

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

@tomdale sgb

@sohara
Copy link

@sohara sohara commented on 5becdc4 Jan 5, 2013

Choose a reason for hiding this comment

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

hilarious

@kylenathan
Copy link

Choose a reason for hiding this comment

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

Example doesnt seem to work - see fiddle http://jsfiddle.net/6NyfN/

Context inside event is the handler itself, not the controller so no controllerFor.

@trek
Copy link
Member

@trek trek commented on 5becdc4 Jan 5, 2013

Choose a reason for hiding this comment

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

My vote is still for App.__c_o_n_t_a_i_n_e_r__

@ramigg
Copy link

@ramigg ramigg commented on 5becdc4 Jan 6, 2013

Choose a reason for hiding this comment

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

@tomdale if there are so many uses of this internal API it means that people don't understand how to tie all the pieces together.

I am confused myself with many aspects, for example, property bindings between controllers, real use cases of view objects which looks like we can handle without them, refresh model data from server and more.

It could be great to have a tutorial for building a real app (not just a basic one as @trek is building, which is very helpful also).

@darthdeus
Copy link
Member

Choose a reason for hiding this comment

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

+1 @ramigg

@tomdale
Copy link
Member

@tomdale tomdale commented on 5becdc4 Jan 7, 2013

Choose a reason for hiding this comment

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

We want this to be painful so that you guys report use cases that are not easily resolved using other methods!

We now provide the needs property on Ember.Controller, which allows you to explicitly declare your dependencies and then have them available on your controllers. Please see 8f60000 for more.

@eibrahim
Copy link

Choose a reason for hiding this comment

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

I am not using ember data and I use simple Ember.Object for my models. I want to lookup my custom store and adapter inside my model, how do I do that? The only way I see this working is through the container.lookup('adapter:xxxx')

Basically, I have a cascadding dropdown situation and I want to load the children when the parent is set on the model. in order to do that I need access to the store and/or adapter. Any ideas? Or am I going about this the wrong way?

@rlivsey
Copy link
Contributor

@rlivsey rlivsey commented on 5becdc4 Dec 2, 2014

Choose a reason for hiding this comment

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

@eibrahim this is better asked on StackOverflow or on Discourse.

You can inject your store into all/specific controllers/routes so it's available from there - you certainly shouldn't need to use the container directly as it's a private implementation detail and it's interface is not guaranteed to stay stable.

Eg I do this with my custom data store in an initializer: https://github.com/rlivsey/fireplace/blob/master/app/initializers/store.js

Coming soon this will be able to be done with services which is currently behind a feature flag on canary.

If you want to go into this some more then please open up a thread on StackOverflow or Discourse as they are much better venues for discussion.

Please sign in to comment.