Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with loading substates on Canary #14585

Closed
jcope2013 opened this issue Nov 8, 2016 · 9 comments
Closed

Issue with loading substates on Canary #14585

jcope2013 opened this issue Nov 8, 2016 · 9 comments

Comments

@jcope2013
Copy link
Contributor

jcope2013 commented Nov 8, 2016

it seems there was a change in behavior between 2.9 and canary with loading substates where declaring the same route name twice, one nested under a different route namespace and one at the top level causes an error with the loading substate

2.9.0 loads fine

https://ember-twiddle.com/fbdb07dd1bcc7817f87643b4a8d435fa

Canary (Uncaught Error: You may not add a duplicate route named posts_loading)

https://ember-twiddle.com/3b08aed8ebc95a5eec495e09ff781274

may be related to #14545

@rwjblue
Copy link
Member

rwjblue commented Nov 8, 2016

This is the error message that was added in tildeio/route-recognizer#118.

/cc @nathanhammond

@rwjblue
Copy link
Member

rwjblue commented Nov 8, 2016

For easier discussion, the Router.map in question is:

Router.map(function() {
  this.route('posts', { resetNamespace: true }, function(){
    this.route('all');
  });

  this.route('users', { resetNamespace: true }, function(){
    this.route('posts', { resetNamespace: true }, function(){
    });
  });  
});

@nathanhammond
Copy link
Member

@jcope2013 You've created undefined behavior with this route map. Where would {{#link-to 'posts'}} take you? Answer: whichever one is defined second, though that is not guaranteed as it's an implementation detail.

This route map is incorrect because there becomes no way to get to /posts except by direct navigation via URL. This is something we consider to be obviously wrong and we want to alert users to their mistake earlier rather than later, thus the addition of the error.

@locks locks closed this as completed Nov 8, 2016
@rwjblue
Copy link
Member

rwjblue commented Nov 8, 2016

@nathanhammond - Maybe we can keep track (i.e. an object with a list of "seen" routes) on Ember's side also so that we can provide a better assertion/error message?

@nathanhammond
Copy link
Member

@jcope2013
Copy link
Contributor Author

jcope2013 commented Nov 9, 2016

so this is what I am trying to do I have a posts.show nested under users (goes to users/posts/show) and a posts.all (goes to posts/all) at the top level, I want to keep both the templates under the posts folder and when linking to posts.show, I want to do {{#link-to "posts.show"}}Post show{{/link-to}}

here it is in 2.9

https://ember-twiddle.com/e23d8ce62b6c298126520ea2827bc748?openFiles=templates.application.hbs%2C&route=%2Fposts%2Fall

On canary, since I get the resetNamespace error, I now need to remove the resetNamespace in the inner posts route nested under users so now I can no longer access the show.hbs under the posts folder while still having the route nested under users, I need to move it into posts/users/show.hbs and I need to replace the {{#link-to "posts.show"}}Post show{{/link-to}} to be instead {{#link-to "users.posts.show"}}Post show{{/link-to}} to get similar behavior

here is the equivalent changes needed to get it working in canary

https://ember-twiddle.com/e786044e2cafeeea95323ab180df515b?openFiles=templates.application.hbs%2C

I guess the trick is I was basing my routes off that second wins behavior and am wondering if there is a similar way to do what I was trying to achieve, mainly

  1. Keep all posts templates under the posts top level folder and not have to nest them under other folders, makes it easier for navigation in my opinion

  2. Keep the posts link in the link-to to be posts.show but still have the url route be users/posts/show instead of the link-to now having to be users.posts.show

@nathanhammond
Copy link
Member

@jcope2013 Do you agree that you were leveraging undefined and hacky behavior previously? Do you believe that pattern should continue to be supported? (I'm asking this quite seriously, I'm curious of your opinion.)

I believe what you're looking for to address Item 1 is this: http://emberjs.com/api/classes/Ember.Route.html#property_templateName

Item 2 I don't believe is worth trying to solve as it confuses what is going on.

@jcope2013
Copy link
Contributor Author

jcope2013 commented Nov 9, 2016

@nathanhammond addressing 1, templateName would fix the issue of where I could keep the templates in the same location but would cause the route file to now be under routes/users/posts/show.js instead of routes/posts/show.js and the controller to be under controller/users/posts/show.js instead of controller/posts/show.js which I feel is a little bit of a downside since I like having the posts folder have all relevant files instead of sprinkling them around, I know I could do some stuff importing and exporting to get around that if necessary.

2 I could live with changing the link-to to have the fully qualified path name such as users.posts.show but the resetNamespace was sort of nice to shorten it up but could go either way there

I previously liked how I could layer resetNamespaces at multiple nested layers and have them all act as top level (both at the link-to/file layer) but if what I was doing was very uncommon then I am not against changing it in my apps, I would be interested to see if any others chime in over the next few weeks once more people test canary/beta once 2.10.0 comes out

@nathanhammond
Copy link
Member

We'll monitor this. I'm pretty strongly of the opinion that combinatorially relying on unspecified and untested implementation details, less-frequently used features, and introducing deliberately broken code paths (what happens in your application when a user directly visits example.com/posts?) should not be presented as a feature. Literally that it works at all is a coincidence as this wasn't intended.

One of the advantages Ember brings is that I can sit down in anybody's project and immediately be productive. Your clever abuse of how Ember resolves modules in combination with how it handles routing subverts that goal except for the most familiar of users.

(You could also set up a custom resolver if you wished.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants