Ember.js: Routes with empty paths steal indexes

[mmun]

Problem

If a non-leaf route (i.e. it has child routes) has an empty path ({ path: '/' }) it will steal the index from its parent.

Examples

An example at the top-level:

Router.map(function() {
  this.route('login');
  this.route('admin', { path: '/' }, function() {
    this.route('dashboard');
  });
});

Twiddle: https://ember-twiddle.com/7bb9deb4fc9e7039d3e5

In this case index will resolve to the full path application.admin.index rather than application.index.

Another example with nesting:

Router.map(function() {
  this.route('login');
  this.route('project', function() {
    this.route('admin', { path: '/' }, function() {
      this.route('dashboard');
    });
  });
});

Twiddle: https://ember-twiddle.com/d5f1693b8c2a66579446

In this case index resolves correctly to application.index, but transitioning to project will resolve to application.project.admin.index rather than application.project.index.

Why is this a bug?

You can convince yourself this is a bug in multiple ways. First, consider what happens when two sibling routes have empty paths:

Router.map(function() {
  this.route('foo', { path: '/' }, function() {});
  this.route('bar', { path: '/' }, function() {});
});

Should index map to application.foo.index or application.bar.index? Trick question! It should map to application.index. Currently the last route wins (application.bar.index here).

Secondly, it's easy to add a redirect from index to admin.index if you want to preserve the current behavior, but implementing a "redirect" the other way is much more painful.

Lastly, this has implications on the isolation of mounted engines, but I won't bother fleshing this point out as engines don't exist yet ;)

[jmurphyau]

I did a bit of analysis and the last route _does_ win.. but not always:

// bar.index wins
Router.map(function() {
  this.route('index', { path: '/' }); // automatically added
  this.route('foo', { path: '/' }, function() {});
  this.route('bar', { path: '/' }, function() {});
});

// bar.index wins
Router.map(function() {
  this.route('foo', { path: '/' }, function() {});
  this.route('bar', { path: '/' }, function() {});
  this.route('index', { path: '/' });  // maybe it could be automatically added at the end? nope.
});


// index.index wins
Router.map(function() {
  this.route('foo', { path: '/' }, function() {});
  this.route('bar', { path: '/' }, function() {});
  this.route('index', { path: '/' }, function() {}); // give it a function and it wins..
});

// index wins
Router.map(function() {
  this.route('foo', { path: '/' }, function() {});
  this.route('bar', { path: '/' }, function() {});
  this.route('index', { path: '' }); //interesting.. and confusing..
});

The router is supposed to favour the route with the least amount of dynamic segments:

From the router.js README:

If there are multiple matches, route-recognizer will prefer routes with fewer dynamic segments, so /posts/edit will match in preference to /posts/:id if both match.

Would it make sense that the router also prefer routes with the fewest number of levels?

Without actually doing the work and testing it I would assume that doing the following could maybe fix this bug:

1. Prefer the least amount of nesting
2. Moving the automatic this.route('index', { path: '/' }) route to after other route definitions

To give an example of scenarios and their results see below. All winner routes are the ones that _would_ win with the above changes.

// a1: dynamic segment (result: /person-a1/list = person-a1.winner)
this.route('person-a1.looser', { path: 'person-a1/:id' });
this.route('person-a1.winner', { path: 'person-a1/list' });

// a2: dynamic segment (result: /person-a2/list = person-a2.winner)
this.route('person-a2.winner', { path: 'person-a2/list' });
this.route('person-a2.looser', { path: 'person-a2/:id' });

// b1: dynamic segment is nested (result: /person-b1/list = person-b1.winner)
this.route('person-b1', function() {
  this.route('looser', { path: '/:id' });
});
this.route('person-b1.winner', { path: 'person-b1/list' });

// b2: dynamic segment is nested (result: /person-b2/list = person-b2.winner)
this.route('person-b2.winner', { path: 'person-b2/list' });
this.route('person-b2', function() {
  this.route('looser', { path: '/:id' });
});

// c1: dynamic segment isn't nested  (result: /person-c1/list = person-c1.winner)
this.route('person-c1', function() {
  this.route('winner', { path: '/list' });
});
this.route('person-c1.loser', { path: 'person-c1/:id' });

// c2: dynamic segment isn't nested  (result: /person-c2/list = person-c2.winner)
this.route('person-c2.loser', { path: 'person-c2/:id' });
this.route('person-c2', function() {
  this.route('winner', { path: '/list' });
});

// d1: all dynamic segment (result: /person-d1/list = person-d1.winner)
this.route('person-d1', function() {
  this.route('loser', { path: '/:id' });
});
this.route('person-d1.winner', { path: 'person-d1/:id' });

// d2: all dynamic segment (result: /person-d2/list = person-d2.loser)
this.route('person-d2.winner', { path: 'person-d2/:id' });
this.route('person-d2', function() {
  this.route('loser', { path: '/:id' });
});

// e1: no dynamic segment (result: /person-e1/list = person-e1.winner)
this.route('person-e1', function() {
  this.route('loser', { path: '/list' });
});
this.route('person-e1.winner', { path: 'person-e1/list' });

// e2: no dynamic segment (result: /person-e2/list = person-e2.loser)
this.route('person-e2.winner', { path: 'person-e2/list' });
this.route('person-e2', function() {
  this.route('loser', { path: '/list' });
});

It looks to only really affect scenarios d1/d2 and e1/e2 - which are relevant to this index issue..

Another idea (but a larger change) could be something like adding this.index() which doesn't allow a path to be passed - and behind the scenes calls this.index('index') and have seperate logic that priorities the routes that are defined as indexes over others..

[mmun]

What is the use case for explicit indexes? They don't fit my mental model at all. Can we add an assert that routeName !== 'index' in this.route(routeName)?

[pixelhandler]

@mmun since the routes describe the UI it's rare for me that 'index' actually describes anything. I always change it, e.g. this.route('list', { path: '/' }) (that page may be a list and have a sibling 'detail' route). Yeah this behavior of which index is actually being rendered seems strange.

[mmun]

btw @jmurphyau Thanks for digging in and exposing some more edge cases.

• I think it's very risky to include nesting in the heuristic. I don't think it is intuitive. The mental model for nesting should be primarily about DOM structure and not about routing.
• What's the usecase for this.index() for? I was thinking of the opposite... deprecating empty paths in this.route and replacing them with something like this.namespace('foo', fn) which roughly expands to this.route('foo', { path: '/' }, fn) but without automatically creating an index.

[mmun]

cc @nathanhammond

[nathanhammond]

@mmun from Slack:

• I expect the behaviour of path: '/' to be different in that it doesn't consume anything in the path - it's for wrapping a set of paths with behaviour

• e.g. authentication

  this.route('foo-auth', { path: '/' }, function() {
  this.route('foo-1');
  this.route('foo-2');
  });
  this.route('bar-auth', { path: '/' }, function() {
  this.route('bar-1');
  this.route('bar-2');
  });
  

• as a user I expect to be able to write something like this and use the foo-auth/bar-auth to house authentication redirects

• and _also_ expect to be able to define a root level index.hbs

[pixelhandler]

@jmurphyau @kitsuneyo @mmun @nathanhammond @toddjordan is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

[mmun]

It's still an issue. The original repro should still be (in)valid.