Conventionalize resource index route's model hook


#1

Just like defining “show” routes in Ember has been conventionalized, it would be nice to conventionalize the index route as well.

For instance, the following pattern has been conventionalized to remove the model hook on the PostRoute:

App.Router.map(function() {
  this.resource('posts');
  this.resource('post', { path: '/post/:post_id' });
});

App.PostRoute = Ember.Route.extend({
  model: function(params) {
    return this.store.find('post', params.post_id);
  }
});

However, the PostsRoute still needs to define the model hook:

App.PostsRoute = Ember.Route.extend({
  model: function() {
    return this.store.find('post');
  }
});

I could understand how conventionalizing finding all records for the PostsRoute and removing the model hook would be too strong of an opinion, but potentially it makes sense on the PostsIndexRoute.

To create the PostsIndexRoute, a function must be passed as an argument:

App.Router.map(function() {
  this.resource('posts', function() {});
});

For which, the PostsIndexRoute would currently look like this:

App.PostsIndexRoute = Ember.Route.extend({
  model: function() {
    return this.store.find('post');
  }
});

I’m proposing to conventionalize this pattern so the PostsIndexRoute would not need a model hook.

The Proposition

I’m actually proposing two things, for which I could submit an initial pull request.

*IndexRoute

Always create the *IndexRoute subroute when defining a resource so you don’t have to pass an empty function:

App.Router.map(function() {
  this.resource('posts');
});

The above would create PostsRoute, PostsIndexRoute (with a default model hook), and prospectively, the related PostsLoadingRoute and PostsErrorRoute.

Fetching

Conventionalize the *IndexRoute hook to function like routes with resource_id dynamic segments by introducing common model hook behavior on current routes like the PostsRoute:

App.PostsIndexRoute = Ember.Route.extend({
  model: function() {
    return this.store.find('post');
  }
});

Thus, removing the model hook so the route looks more like:

App.PostsIndexRoute = Ember.Route.extend({});

Which, of course, allows us to omit the PostsIndexRoute definition altogether.


#2
  • This may influence application performance by adding unnecessary routes
  • Also, this will mean that it will ‘flood’ Ember Inspector with virtual routes that are never used.

On other hand this may be a good improvement for Ember beginners, to help to avoid some potential problems.

I personally don’t see a problem writing function() {} each time.

Again, I aggree, this could be good help for Ember beginners.

Would it make ‘index’ model as default model for error and loading routes as well?

Currently this can be achieved by doing something like:

App.PostsRouteHelper = Ember.Route.extend({
  model: function() {
    return this.store.find('post');
  }
});

App.PostsRoute = App.PostsRouteHelper.extend();
App.PostsIndexRoute = App.PostsRouteHelper.extend();

#3

Having to add function(){} certainly bit me in the ass a few times when I started with ember. It’s decidedly non-obvious and I can’t see any value in having it defined this way.

It actually reminds of another place in the framework where things aren’t generated as you’d expect. If you use render and additional parameters you can use them in the view, but only if you define a view.

If we’re going to generate default implementations it should be done consistently otherwise when you’re starting out it feels like a bit of a crapshoot.


#4

@mikepack and @muchweb Could you please correct your post from saying store.find(‘posts’) to store.find(‘post’) for the index? Unless for some reason my brain has somehow dropped the way that Ember Data works? (horrified look) :slight_smile:


#5

@JulianLeviston, good point :thumbsup:


#6

Thank you, muchly :smile: It’s helpful for new people.


#7

@JulianLeviston Apologies for the typos and poor formatting in my original post. Should be fixed now.

@muchweb

Good thought. You’re right, and your point about helping beginners is important. People could learn about the virtual routes through the Ember Inspector. Currently, you’d have to stumble upon the documentation or source to find that PostsIndexRoute, PostsLoadingRoute and PostsErrorRoute are possible.

Possibly this could be alleviated with a setting in the Inspector to “show all default routes” or “show all subroutes” or the like.

I’m not sure, but I’m inclined to say “no.” Since fetching all posts (in this example) is a fairly expensive task, I think the introduction of this convention should be conservative. Further, error and loading are rather exceptional; I’m not convinced the model would be relevant in these contexts.


#8

I’m fairly new to Ember development, but I would vote not to conventionalize this because I almost never use a “raw” this.store.find('posts'), but rather “scope” it using a where, or pagination, etc.