Skip to content
This repository has been archived by the owner on Mar 22, 2019. It is now read-only.

Guide to Object management and dependency injection in Ember #1293

Closed
wants to merge 13 commits into from

Conversation

pksjce
Copy link
Contributor

@pksjce pksjce commented Feb 13, 2014

I saw @mixonic s talk at http://www.youtube.com/watch?v=iCZUKFNXA0k#t=6318
This is how I understood the Ember container to be and thought new people trying to look inside Ember should know it too.

@stefanpenner
Copy link
Member

although the container exists, for the most part it should be considered an implementation detail.

The public api is actually only

app.inject
app.register

I would prefer for today that these are the API's we highlight

It is likely fine, to show people how the container does the magic, but I would like to convey a clear message,

```js
App.IndexView = Ember.View.extend({
router: function(){
return this.container.lookup('router:main');
Copy link
Member

Choose a reason for hiding this comment

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

container.lookup exist to enable escape hatches, for ember API deficiencies. In lots of cases, and initializer with an injection rule is much superior. This is because it also performs validation as the object is instantiated, provided quick-fail feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, But I would need to add documentation on initializers for that right? And mention the correct usage for lookup?

Ember.Application.initializer({
  name: "store",
  initialize: function(container, application) {
    container.register('store:main', application.Store);
    container.injection('controller', 'store', 'store.main');
  }
});

Like this? ^

@pksjce
Copy link
Contributor Author

pksjce commented Feb 13, 2014

Shall I add an FAQ similar to the run loop page where I put up info about App.inject and App.register ?

@@ -369,6 +369,8 @@ Understanding Ember.js:
url: "understanding-ember/debugging"
- title: "The Run Loop"
url: "understanding-ember/run-loop"
- title: "Object Management and Dependency Injection"
Copy link
Member

Choose a reason for hiding this comment

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

I might say "Service Lookup and Dependency Injection", though it is more technical than "Object Management". "Object Management" just strikes me as a vague title.

@mixonic
Copy link
Member

mixonic commented Feb 13, 2014

@pksjce So though my talk dived deep into how the container powers Ember internals, there is a core team consensus that the API is not extremely stable. Because of this, they would like to steer people away from using the container API directly. For a website guide, it is probably best to give the specifics of Ember.Container a broad brush and move on.

What this means is that a guide section for Service Lookup and Dependency Injection might better focus on these container-powered APIs:

  • needs:, which does dependency injection of controllers
  • App.inject and App.register

I think approaching the guide with a pattern focus instead of an instruction focus may be ideal.

  • Create a service with controllers (needs:)
  • Injecting a new service onto all items of a type (App.register and App.inject, how to make something like this.store)
  • Demonstrate the benefit of DI in tests.

Thanks for the pass at this! Let me know if you want more help on it to move it forward.

@pksjce
Copy link
Contributor Author

pksjce commented Feb 13, 2014

I can do that. Let me have another go at this. I'll notify you on my next PR soon enough.
Thanks!

@stefanpenner
Copy link
Member

can you please re-use this existing PR, it will make it much easier to track the comments and improvements.

@lukemelia
Copy link
Member

Hmm, container.lookup is not public API @stefanpenner? That means that there is no public API for service location. Do we really believe you can build real-world Ember apps today without using that pattern on occasion?

@stefanpenner
Copy link
Member

@lukemelia container.lookup directly is likely the only public api on container. Remember it exists as an escape hatchmput in place to allow for scenarios the current API's lack proper solutions for.

It's use should not be encouraged, as typically it results in more brittle failures, then it's injection based counter-parts.

@ebryn
Copy link
Member

ebryn commented Mar 18, 2014

Let's drive this to completion

@pksjce
Copy link
Contributor Author

pksjce commented Mar 19, 2014

@mixonic - I tried to stick to what you had advised. Have documented needs, inject/register and initializers. I'm a bit unsure of the usage of DI in testing. Can you have a look at this http://emberjs.jsbin.com/qafat/1/edit?html,js,console,output
I need some help to come up with better examples I think.

@mixonic
Copy link
Member

mixonic commented Mar 21, 2014

@pksjce will review. Thanks for taking another pass!!

fwiw I've improved the documentation for App.register and App.inject in the API emberjs/ember.js#4577

Rewrite some early sections of the dependency injection guide
@pksjce
Copy link
Contributor Author

pksjce commented Mar 24, 2014

@mixonic - Thanks very much for the edit! Inline examples while explaining theory was good idea. So, do you suggest removing explanations on Application initializers?

@mixonic
Copy link
Member

mixonic commented Mar 24, 2014

@pksjce I think using initializers for the App.register and App.inject section would be fine, even idiomatic. But we should avoid talking about the container API in this guide for now.

##### Simple Dependency Injection with needs

A common use-case for dependency injection is that of a singleton service. For
instance, a controller maintaining the session state may exposed to many other
Copy link
Contributor

Choose a reason for hiding this comment

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

may be exposed

@pksjce
Copy link
Contributor Author

pksjce commented Mar 27, 2014

Okay. I made the corrections.

Revamp the audio demo and Crosslink docs from controllers to DI guide
@pksjce
Copy link
Contributor Author

pksjce commented Apr 15, 2014

Hey. Any update?

@hswolff
Copy link

hswolff commented Apr 21, 2014

Hoping we can get this merged in soon!

});
```

Now, to register it
Copy link
Member

Choose a reason for hiding this comment

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

this must occur in an initializer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stefanpenner - That means something like

Ember.Application.initializer({
initialize: function(container, application) {
container.register('logger:main' ...
}
});

This documentation does not have information about using container object right?

Copy link
Member

Choose a reason for hiding this comment

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

You can use:

Ember.Application.initializer({
  initialize: function(container, application){
    application.register('logger:main', /* ...whathaveyou... */);
  }
});

application has .register and .inject.

Copy link
Member

Choose a reason for hiding this comment

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

ping

@mixonic
Copy link
Member

mixonic commented Apr 22, 2014

Touched on some of @JulianLeviston's feedback in pksjce#3, but the last section still seems to need some polish. Will find more time to edit/write this week.

@mixonic
Copy link
Member

mixonic commented Jun 29, 2014

@pksjce I'm going to fork this into a new pull request, since I presume you don't have further interest in pushing it forward. I think it will save us both a bunch of time.

@mixonic
Copy link
Member

mixonic commented Jun 30, 2014

Forked to #1581

@rwjblue
Copy link
Member

rwjblue commented Jun 30, 2014

Closing in favor of #1581.

@rwjblue rwjblue closed this Jun 30, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants