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

router-scroll location type breaks non-root routes in development #21

Closed
kpfefferle opened this issue Aug 10, 2016 · 12 comments
Closed

Comments

@kpfefferle
Copy link

kpfefferle commented Aug 10, 2016

Create new ember-cli app:

  1. ember new router-scroll-test
  2. cd router-scroll-test
  3. Remove ember-welcome-page addon
  4. ember generate route nested
  5. Add content to app/templates/nested.hbs
  6. ember serve
  7. Visit localhost:4200/nested (content loads fine)

image

Introduce ember-router-scroll:

  1. ember install ember-router-scroll
  2. Follow README setup (import mixin into router.js; set locationType: 'router-scroll')
  3. Restart development server with ember serve
  4. Visit localhost:4200/nested

image

It seems that something in the 'router-scroll' locationType is breaking the development express server's ability to load the app directly into a non-root route. This is especially frustrating when livereload is constantly refreshing the page during development (triggering this error each time).

@bennycwong
Copy link
Contributor

Hi @kpfefferle. Thanks for letting us know. I am able to reproduce it.
@bcardarella is working with the ember team to get the router-scroll location API stuff into mainline ember, so I think after that, it might resolve itself, hopefully.

@bcardarella
Copy link
Member

This is really strange. I can see the route exists in Inspector. Still digging.

This isn't happening on my local branch of dockyard.com but I clearly see it in this demo app

@bcardarella
Copy link
Member

I suspect that the HistoryLocation is not working properly after being extended

@bcardarella
Copy link
Member

I believe this is the source of the bug: ember-cli/ember-cli@1171c3d

@bcardarella
Copy link
Member

tldr; ember-cli's HistorySupport middleware that is used to serve up the baseUrl with any nested urls is hard coded to limit to auto and history types.

Short-term solution: we can reopen Ember.HistoryLocation until emberjs/ember.js#14011 is merged in and released. (a few months from now)

@RobbieTheWagner
Copy link
Contributor

@bcardarella do you have plans of dropping in the solution you mentioned? Would love to not have to go back to my root route every time I live reload 😄

@bcardarella
Copy link
Member

I posed it as a possible solution. I'm not the maintainer of the library and was awaiting feedback on the preferred direction.

@bennycwong
Copy link
Contributor

bennycwong commented Aug 18, 2016

@kpfefferle @rwwagner90 @bcardarella I finally had a chance to look at this. I was able to get the nested route working by adding this to config/environment.js

    locationType: 'router-scroll',
    historySupportMiddleware: true, // force the use of the history location middleware

@bennycwong
Copy link
Contributor

I'm able to get the router scroll working with live reload in nested routes:

demo

@bcardarella
Copy link
Member

Sweet!

On Thursday, August 18, 2016, Benny C. Wong notifications@github.com
wrote:

Closed #21
#21.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#21 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABIXCtfSFJWfbJAJbzpXIxl6oPI9MMXks5qhM-pgaJpZM4JgsG5
.


Brian Cardarella
CEO of DockYard
Visit us: http://dockyard.com
Call us: (855) DOCK-YRD
Follow me on Twitter: http://twitter.com/bcardarella
Follow us on Twitter: http://twitter.com/DockYard

@kpfefferle
Copy link
Author

@bennycwong I can confirm that adding historySupportMiddleware: true to config/environment.js fixes the issue for our app as well. Perhaps this should be added to the README setup instructions before the issue is considered resolved entirely? If it's not communicated there, then it may continue to cause frustration for future installs.

Thanks for tracking down a workaround!

@bennycwong
Copy link
Contributor

bennycwong commented Aug 18, 2016

#28 has the instructions in the readme. I'm having a hard time explaining how to concisely explain why that config is needed, so I just linked to the issue.

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