Skip to content

Make trailing slash optional #820

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

Closed
aldendaniels opened this issue Feb 11, 2015 · 32 comments
Closed

Make trailing slash optional #820

aldendaniels opened this issue Feb 11, 2015 · 32 comments

Comments

@aldendaniels
Copy link

Desired behavior:

  1. Always allow an optional trailing slash
  2. Never add an optional trailing slash when generating links via makePath

I think this behavior is pretty standard.

Today (as of v0.12.9) makePath appends a trailing slash if a trailing slash is optionally allowed.

Food for thought (tangential): It would be nice to support optional trailing slashes automatically without the need to add a /? to all routes.

@agundermann
Copy link
Contributor

For me, the desired behavior would be

  1. Force trailing slash for routes (via redirect), but only for routes with children/DefaultRoutes ("directories")
  2. Always add trailing slash to links to those routes

So I don't think changing the default behavior is the ideal solution. But it would be nice to have some more control over this.

@mjackson mjackson changed the title makePath() should omit optional trailing slash Make trailing slash optional Feb 22, 2015
@jkimbo
Copy link

jkimbo commented Mar 22, 2015

+1

1 similar comment
@romeovs
Copy link

romeovs commented Apr 9, 2015

+1

@mikepro4
Copy link

+1

@Frikki
Copy link

Frikki commented Apr 24, 2015

This is really an important issue. However, since the issue hasn’t been resolved, what is the workaround?

@aronwoost
Copy link

+1

1 similar comment
@super-cache-money
Copy link

+1

@mbektimirov
Copy link

Yep, very annoying issue. Path must be handled with and without trailing slash by default as it is not classic REST router.

@dignifiedquire
Copy link

@ryanflorence @mjackson will this be addressed/changed in #1158? Does it make sense to implement it on the current code base, or should I rather start based on the code in #1158 ?

@mjackson
Copy link
Member

@dignifiedquire This is already fixed in #1158

@fxxkscript
Copy link

@mjackson how to solve this issue now?

@ryanflorence
Copy link
Member

its now optional in 1.0

@kevzettler
Copy link

workaround for this in 0.13.2 ?

@fxxkscript
Copy link

@kevzettler add /? to the url

@TimDaub
Copy link

TimDaub commented Jul 21, 2015

workaround for this in 0.13.2 ?

+1

@rtm7777
Copy link

rtm7777 commented Jul 23, 2015

/? is not work in 1.0.0-beta3?

@tommmyy
Copy link

tommmyy commented Aug 28, 2015

It is not working in Link component. Class "active" is not added in case of trailing slash.

In the method Router#isActive are compared wrong strings:

to === state.path

which in my case of using /? in route's path looks like this:

"/about" === "/about/"

@th0r
Copy link

th0r commented Aug 28, 2015

Hi guys! Can't you just redirect every path with trailing slash to the path without it on server (using express for example)?
It completely solved this issue for us.

@tommmyy
Copy link

tommmyy commented Aug 28, 2015

@th0r yes I ended up with unification of all routes

@chibicode
Copy link

(On 1.0.0-rc3) This is strange, but when using IndexRedirect, having a trailing slash on the root path caused a problem for me.

If I do this (a slash after admin):

  <Route path='/admin/' component={App}>
    <IndexRedirect to='posts' />

Then if I open a new window and go to localhost:.../admin/ it gets redirected to /admin/posts correctly, BUT if I open a new window and go to localhost:.../admin (without a slash at the end) it gives me an error:

Warning: Location "/admin" did not match any routes

To fix this, I had to remove the trailing slash from the root component:

  <Route path='/admin' component={App}>
    <IndexRedirect to='posts' />

I wanted to write a failing test but couldn't figure out how:
https://github.com/rackt/react-router/blob/c025177a10785f96f7683d4f033f6acdd46c1f2e/modules/__tests__/IndexRedirect-test.js#L20-L31

If someone could help me write a failing test, I'd create a new issue.

@taion
Copy link
Contributor

taion commented Oct 27, 2015

Just don't specify the trailing slash on path if you want to match the slash-less route - easy enough.

@tomconroy
Copy link

For anyone wondering about how to do this in express:

// remove trailing slashes
app.use(function(req, res, next) {
  if (req.path.length > 1 && /\/$/.test(req.path)) {
    var query = req.url.slice(req.path.length)
    res.redirect(301, req.path.slice(0, -1) + query)
  } else {
    next()
  }
})

@rhzs
Copy link

rhzs commented Jan 7, 2016

any update on this feature?

@romidane
Copy link

romidane commented Feb 6, 2016

I found a solution that worked for me
"I added <base href="https://github.com/" /> into the <head> of my index.html and it worked (:"

Source:
http://stackoverflow.com/questions/28253162/react-router-dynamic-segments-crash-when-accessed

@cmmartti
Copy link

cmmartti commented Feb 8, 2016

@Frikki: Old, I know, but I saw this somewhere. This will force all routes to end in a slash.
<Redirect from="/*" to="/*/" />
You need to import Redirect from 'react-router'.

@cansin
Copy link

cansin commented Sep 30, 2016

I'd love what @cmmartti worked, but it is causing an infinite loop for unknown paths as far as I understand.

@MilllerTime
Copy link

MilllerTime commented Oct 28, 2016

I'm using react-router v2 and was able to force trailing slashes using onEnter and onChange hooks. All of my URLs are "directory" style, so I always add a trailing slash. If you need more sophisticated logic, this approach gives you the full power of JS to do so.

The technique relies on the fact that you can wrap all existing <Route>s with a path-less <Route> without affecting your application. By adding hooks to the new top-level route, it catches all changes with no ill-effects.

My <Router>:

<Router history={browserHistory}>
  {/* ↓ This route hooks up the magic, all else can be replaced with your own routes */}
  <Route onEnter={forceTrailingSlash} onChange={forceTrailingSlashOnChange}>
    <Route path="/" component={MainLayout}>
      <IndexRoute component={Dashboard} />
      ...other routes
    </Route>
  </Route>
</Router>

forceTrailingSlash checks if the new path has a trailing slash and if not uses replace() to add a trailing slash while preserving all other route data.

function forceTrailingSlash(nextState, replace) {
  const path = nextState.location.pathname;
  if (path.slice(-1) !== '/') {
    replace({
      ...nextState.location,
      pathname: path + '/'
    });
  }
}

forceTrailingSlashOnChange simply delegates to forceTrailingSlash, which is necessary because the onChange hook has an extra argument we don't need.

function forceTrailingSlashOnChange(prevState, nextState, replace) {
  forceTrailingSlash(nextState, replace);
}

@pugnascotia
Copy link

Is there a solution for optional trailing slashes for v4?

(sorry for notification spam - deleted the first comment by accident)

@cristianele
Copy link

Simple

<Route path='/admin(/)' component={App}>

@MilllerTime
Copy link

MilllerTime commented Feb 9, 2017

@pugnascotia From my understanding of the v4 design, the approach I described would still apply with light refactoring:

  • instead of wrapping your app in a <Route> to track changes, you'd just use a custom component
  • replace the onEnter and onChange hooks with standard component lifecycle methods componentWillMount and componentWillReceiveProps.

componentWillMount is called on the server as well, and should work for universal apps. I'm not 100% sure these are the best lifecycle methods to replace RR's hooks, but I'm sure a future migration guide will make that clear.

@cristianele That's not a DRY solution for multiple routes. Also, isn't it good practice to support only a single URL for a given page of content? You're making a trailing slash optional for users, but also allowing two equivalent URLs for each route with that treatment.

@lewisdiamond
Copy link

For those of who are interested in forcing a trailing slash on specific routes:

<Route path="/:id(\d+)" exact strict render={props => <Redirect to={`${props.location.pathname}/`}/>}/>

This is nice if a user goes directly to /123 but your links are like <Link to={item}/> to go to /123/item

@thomasw
Copy link

thomasw commented Jul 7, 2017

Following up on @lewisdiamond's solution (using 4.1.x specifically), I believe enforcing trailing slashes for all routes can be achieved by making the following the first route in your router:

<Route path="/:url*" exact strict render={props => <Redirect to={`${props.location.pathname}/`}/>}/>

/:url* and strict match any number of /urlfragments but this combination does not match the root url or any paths with a trailing slash. The render method issues a redirect that appends a / to the current path as in @lewisdiamond's post.

Sample root component:

  render() {
    return <Provider store={this.props.store}>
        <Router history={this.props.history}>
          <Switch>
            <Route exact strict path="/:url*" render={props => <Redirect to={`${props.location.pathname}/`}/>} />
            <Route exact path="/login" component={Login} />
            <Route exact path="/logout" component={Logout} />
            <Route path="/" component={MarketingSection} />
            <Route path="/Subsection" component={SomeSubsectionComponent} />
            <Route component={FourOhFour} />
          </Switch>
        </Router>
      </Provider>
  }

This change shouldn't require modifying routes anywhere else in an app as long as no routes strictly required no trailing slash (those would now be broken). I was able to drop this redirect into an existing app that was both defined as in the example above and exclusively used slashless internal links. Everything just worked with one exception: At one route in particular, there was some bad url join logic that assumed there'd be no trailing slash in the path and it only worked correctly in that case (meaning that the accessing that route with a trailing slash was broken before my change anyway). Be on the lookout for fun stuff like url.resolve(currentPath + '/', someID).

Note that non-matching routes of the form /some/bad/route are redirected to /some/bad/route/ before displaying the FourOhFour component in the example above. If one were to enforce trailing slashes at a web server level, this is very similar to the behavior you'd see there (web server redirects /some/bad/route to /some/bad/route/ and then passes the request off to your application code which then responds with a 404).

@remix-run remix-run deleted a comment from leite08 Dec 4, 2017
@remix-run remix-run deleted a comment from bill-revcascade Dec 13, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests