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

Index route (or rather, route at '/') with nesting breaks routing in ember 2.7 #13960

Closed
acburdine opened this issue Jul 29, 2016 · 7 comments
Closed

Comments

@acburdine
Copy link

So I was trying to update Ghost to Ember 2.7, and I ran into a fairly large issue with ember's routing that breaks the application.

The scenario: Ghost has a route called posts, with a path of / and a nested route inside of it (post) that has the path :post_id. Ghost also has several other routes like team (/team), setup (path: /setup), etc., some with nesting and some without.

Prior to Ember 2.7, this worked just fine, but after trying to upgrade, each explicit route (e.g. /team, /setup, etc.) is now hitting the posts.post route, and being treated as a post id.

Ghost's router.js: https://github.com/TryGhost/Ghost-Admin/blob/master/app/router.js

I also built a couple of Twiddles to give a simpler example.

Ember 2.6 - https://ember-twiddle.com/cfbaa6316214d725c87be3e80c131d82

  • type in to the route input /5 (or anything other than /team)
    • notice that an alert appears saying "Post Route with id: 5" (or whatever you entered)
  • type in to the route input /team
    • notice that an alert appears saying "Team Route"

Ember 2.7 - https://ember-twiddle.com/0da9cf763f86ce395bfbfe7072e43c53

  • type in to the route input /5 (or anything other than /team)
    • notice that an alert appears saying "Post Route with id: 5" (or whatever you entered)
  • type in to the route input /team
    • notice that an alert appears saying "Post Route with id: team"

I wasn't sure if this was unexpected breaking behavior, or if Ghost's router is doing something wrong. If Ghost's router is doing something wrong, it would be awesome to figure out a way to fix it 😄

@acburdine
Copy link
Author

Note: I believe it's related to this PR

@nathanhammond
Copy link
Member

@acburdine we know what this problem is, and it's us, not you. There will be a 2.7.1 release addressing this bug. I don't know where to tell you to follow along but know it's an ongoing discussion. In the mean time I suggest you stick with 2.6. 😄

@acburdine
Copy link
Author

@nathanhammond thank you for the quick reply!

@mixonic
Copy link
Member

mixonic commented Aug 6, 2016

@acburdine does this issues cause the ghost tests to fail? If I can get you a build with our proposed fix, can you run those tests easily or can I?

@acburdine
Copy link
Author

acburdine commented Aug 6, 2016

@mixonic the straight upgrade to ember 2.7 triggered the errors - both in tests and in actual running of the app - you should be able to run the tests easily 😄

Although if you want me to run them just let me know

@mixonic
Copy link
Member

mixonic commented Aug 7, 2016

I've uploaded a production build of Ember with proposed fixes here:

There is a full ember build in that dir, so min.js and prod.js are also there. I'll attempt to repro the issue with Ghost and confirm this fix.

@mixonic
Copy link
Member

mixonic commented Aug 7, 2016

Neat, I've confirmed both the original failure in tests and that the above build fixes this regression.

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

No branches or pull requests

4 participants