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

Add initializers guide #1779

Closed
wants to merge 1 commit into from
Closed

Conversation

opsb
Copy link

@opsb opsb commented Nov 4, 2014

This PR adds an Initializers section to the guides.

@rwjblue
Copy link
Member

rwjblue commented Nov 4, 2014

@trek - Can you review?

});
```

### Asynchronous initializers
Copy link
Member

Choose a reason for hiding this comment

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

nope, these are extremely dubious, please use your routes to absorb this async... Please remember this exists from a time before routes and routers.

If anything we should explicitly say advance/defer is discouraged and will go away

@stefanpenner
Copy link
Member

other then async in initializers this looks good.

@opsb
Copy link
Author

opsb commented Nov 4, 2014

@stefanpenner I went ahead and removed the async section

@stefanpenner
Copy link
Member

thanks looks good

@opsb
Copy link
Author

opsb commented Nov 4, 2014

I've never actually found a use for the application parameter except for the async control, is there any other use worth mentioning?

@stefanpenner
Copy link
Member

@opsb yes, it should be used instead of the container.

Currently the only public api's for the container are actually through the application

application.inject
application.options
application.register

everything else is private

application.options is also currently broken :( Someone needs to fix that..

using looku will leave your app and addon extremely brittle and will likely be removed sometime soon in-favor of

container.afterInitialize(fullname, function(instanceThatWasJustInitialized) {

});

that then leaves lookupFactory which is likely ok but need some more thought

@opsb
Copy link
Author

opsb commented Nov 4, 2014

@stefanpenner presumably you still need to use container for lookup though or is there an equivalent on application?

@opsb
Copy link
Author

opsb commented Nov 4, 2014

As it's a common use case I've added a section on registering services.

@stefanpenner
Copy link
Member

As it's a common use case I've added a section on registering services.

these seems resolvable, as they all follow conventional naming and the default + ember-cli resolvers should both handle this correctly.

You will run into the fact that application.options('type:name', { instantiate: false }) incorrectly applies the options to only type when it should actually be applied to `type:name'. But this is a ember-bug.

@stefanpenner
Copy link
Member

@stefanpenner presumably you still need to use container for lookup though or is there an equivalent on application?

You do not need to lookup in your initializers.

This is quite problematic when people do this, as it changes the natural boot order of things and makes add ons basically impossible (or at best extremely tedious as the dag ordering must be updated)

@opsb
Copy link
Author

opsb commented Nov 4, 2014

@stefanpenner so is best practice to not touch container at all now then? In that case I'll remove the example that lookups up the ember-data store. In fact should I caution that container should not really be used anymore?

@mixonic
Copy link
Member

mixonic commented Nov 4, 2014

There is already a dependency injection guide, please review to make sure there isn't too much overlap! Maybe they should be cross-referencing.

@opsb
Copy link
Author

opsb commented Nov 4, 2014

@mixonic I just added a brief example and linked to the dependency injection guide for further information.

@stefanpenner
Copy link
Member

It should not have been used originally. It was an omission leaving it as part of the initializer api

@opsb
Copy link
Author

opsb commented Nov 4, 2014

@stefanpenner I've removed any reference to container in the examples and there's a note explaining that it should not be used. Examples now all use the form:

Ember.Application.initializer({
  name: "initializerName",

  initialize: function(_, application) {
    // your initialization code
  }
});

@opsb
Copy link
Author

opsb commented Nov 5, 2014

@stefanpenner @mixonic @trek are there any other changes you'd like me to make on this or can we get it merged in?

@stefanpenner
Copy link
Member

@opsb this looks great to me. But i'll need @trek or @mixonic to +1 since this is written english and not code :P

@trek
Copy link
Member

trek commented Nov 5, 2014

Was there some reason this is a guide instead of the preamble text for Ember.Application.initializer? We're trying to move away from guides that explore multiple options because they tend to expand and become very wordy.

@opsb
Copy link
Author

opsb commented Nov 5, 2014

@trek I think the api docs are great once you know your way around ember but if you're new to the framework the guides provide a far better structure for getting up to speed. The rails guides do an excellent job of guiding people through the framework and I'd like to see the ember guides get to that level (docs are frequently cited as a problem with ember even though there's plenty of them).

edit: would you prefer the page to be split out into multiple pages, one for each section?

@trek
Copy link
Member

trek commented Nov 7, 2014

The rails guides do an excellent job of guiding people through the framework and I'd like to see the ember guides get to that level

There is indeed a fundamental discoverability problem with the API docs, but that is the root problem. The solution is fixing the API.

We've gone back and forth on appropriate guide size and structure but have never really reached consensus on the core team or in the community other than "guides should be short and focused on introducing the topic and how it relates to other high-level topics, then heavily cross-linked with the API docs"

We're already in a place where the guides have gotten away from us, quality wise, and the current plan is to significantly trim down what is in the guides as we move towards 2.0, leaving API documentation as the correct location for longer, in-depth exploration of various uses of each part of the framework.

would you prefer the page to be split out into multiple pages, one for each section?

I think, based on what consensus could be reached about guides/docs, that an initializer section describing what they are and how they work with other parts of the framework is good. Sections like Initializers for multiple ember apps and Initializing services need to be part of API docs.

I'm not convinced that leads to the best docs, but we have to pick some direction and go with it. Right now the guides are a patchwork of styles, length, and goals that ultimately overall isn't up the quality we'd like.

@opsb
Copy link
Author

opsb commented Nov 12, 2014

We're already in a place where the guides have gotten away from us, quality wise, and the current plan is to significantly trim down what is in the guides as we move towards 2.0, leaving API documentation as the correct location for longer, in-depth exploration of various uses of each part of the framework.

@trek, @stefanpenner I'm not sure that I agree that in-depth exploration of uses should be in the API. I've always considered a guide to document how to do something and the API to document what you can do with the various classes/methods. Crucially the guide is organised to match the user's use cases(e.g. how do I create an app, model, save to the db etc.), something which I believe the current guides do, but in an incomplete fashion. To illustrate the point, consider that there is a high degree of similarity between the organisation of guides from different frameworks but the API(and accompanying documentation) is often entirely different.

Personally I'd like to see a far easier on ramp for ember, which for me means improving the guides rather than the API documentation. I'm very happy to contribute to this effort but currently it feels like there isn't much of an appetite to do so (I have a couple of other open PRs intended to make it easier to get started with ember-data and rails, #1634 and emberjs/data#2138). I can appreciate that moving the framework forwards (2.0 looks awesome!) is the priority but I feel that there's some low-hanging fruit where improvements to the guides could make a big difference to new-comers.

@trek
Copy link
Member

trek commented Nov 12, 2014

I'm not sure that I agree that in-depth exploration of uses should be in the API.

There's already been a ton of back and forth on the subject with no option being the clear winner, so we went with succinct guides/expanded API. If that doesn't work out, we can change course, but we have to go with something and that's the decision that was made.

For every person who preferred Rails Guide style, there was someone else who hated it and said the length made it difficult get answers.

@locks
Copy link
Contributor

locks commented Feb 22, 2015

Ping for status on this PR.

@stefanpenner
Copy link
Member

@dgeb can you review this now that we have introduced the registry?

@locks thanks for the ping, we likely need to update since the recent container/registry split.

@locks
Copy link
Contributor

locks commented Mar 20, 2015

This now needs to be PRed against emberjs/guides and rewritten for CLI, correct?

@trek
Copy link
Member

trek commented Mar 20, 2015

Yes, this would need to target https://github.com/emberjs/guides now and be updated to reflect cli-style structure.

@trek
Copy link
Member

trek commented Mar 27, 2015

We've migrated to a versioned guides project. Please reopen on https://github.com/emberjs/guides. Thanks!

@trek trek closed this Mar 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants