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

Force urls to have a trailing slash #89

Open
RobbieTheWagner opened this issue Feb 7, 2018 · 7 comments
Open

Force urls to have a trailing slash #89

RobbieTheWagner opened this issue Feb 7, 2018 · 7 comments

Comments

@RobbieTheWagner
Copy link
Contributor

I have been working on converting my app to use prember, and I had a need to force trailing slashes. I am extending the router-scroll location type to achieve this. Would there be interest in adding support here directly with a flag to enable?

import EmberRouterScroll from 'ember-router-scroll/locations/router-scroll';

export default EmberRouterScroll.extend({
  formatURL() {
    let url = this._super(...arguments);

    if (url.includes('#')) {
      return url.replace(/([^/])#(.*)/, '$1/#$2');
    } else {
      return url.replace(/\/?$/, '/');
    }
  }
});
@yowainwright
Copy link
Contributor

@rwwagner90 I could see scenarios where this would cause issues.

What is the benefit?

Also, if this is a feature that the community wants, using includes would probably cause issues.


@RobbieTheWagner
Copy link
Contributor Author

@yowainwright the benefit is to require slashes in urls for static sites. I'm using prember, which needed this tweak.

includes is totally fine and supported in node 4+. Ember is even dropping node 4 at the end of April.

@lougreenwood
Copy link
Contributor

I've just come across this issue too, @rwwagner90 👍

@yowainwright for context, the issue for me is an SEO one - omitting the trailing slash means that Google isn't discovering the actual page.

@RobbieTheWagner
Copy link
Contributor Author

Ember has now dropped node 4 support as well, so this should be good to go. If you guys are open to it, I can open a PR.

@snewcomer
Copy link
Collaborator

@rwwagner90 I'm guessing this is a global flag?

@RobbieTheWagner
Copy link
Contributor Author

RobbieTheWagner commented Sep 6, 2018

@snewcomer yeah, would need a way to turn it off and on

@RobbieTheWagner
Copy link
Contributor Author

@snewcomer any thoughts on if we want to implement this? At the very least, we could document how to do this in the README.

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