Skip to content

Specifying default sub-routes and redirecting when there is no activeRouteHandler match #149

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
JedWatson opened this issue Jul 30, 2014 · 21 comments

Comments

@JedWatson
Copy link
Contributor

Not sure if I missed something obvious about how to do this, but here's what I'm trying to achieve:

Say there's a User route that has two sub-routes: UserDetails and UserActivity.

The User route draws common UI (name, navigation, etc) and needs to know the userId in order to do so.

So I set them up like this:

<Router.Routes>
  <Router.Route handler={App}>
    <Router.Route name="user" path="user/:userId" handler={User}/>
      <Router.Route name="user-details" path="user/:userId/details" handler={UserDetails} />
      <Router.Route name="user-activity" path="user/:userId/activity" handler={UserActivity}/>
    </Router.Route>
  </Router.Route>
</Router.Routes>

App draws the surrounding UI, User draws the common UI for the current user (name, subnav), and UserDetails and UserActivity draw their specific UI.

The User view doesn't have any 'inner' UI and requires either the UserDetails or UserActivity view to be rendered inside it, but it seems reasonable that a) someone may visit the url /user/1, and b) the default redirect should be encapsulated somewhere.

What I can't work out is how to best ensure that any requests to /user/1 either render the UserDetails handler by default, or automatically redirect to /user/1/details.

I've (sort of) been able to hack it by adding a willTransitionTo static on the User component, like this:

willTransitionTo: function (transition, params) {
  if (transition.path.split('?')[0] === '/user/' + params.userId) {
    transition.redirect('user-details', params);
  }
}

...but that's messy, and breaks the encapsulation of the routes / paths, among other things.

It seems that there are a few additions to the API that could make it cleaner, including:

  • Add a property to Route that lets you specify a default sub-route handler (these could cascade, and wouldn't cause a redirect)
  • Add a property / argument / etc. available in willTransitionTo that indicates this is the 'final' matched route; i.e. no child routes will also be rendered

Maybe a nice thing to have available generally would be an array of the matched routes, starting with the upper parent; this would be useful in both willTransitionTo and willTransitionFrom.

Alternatively, I may be going in completely the wrong direction here; If I could get the :userId param into the User route without specifying the path, I could make the path of the user-details route /user/:userId, and that would be fine.

@ryanflorence
Copy link
Member

<Router.Routes>
  <Router.Route handler={App}>
    <Router.Route name="user" handler={User}/>
      <Router.Route name="user-details" path="user/:userId/details" handler={UserDetails} />
      <Router.Route name="user-activity" path="user/:userId/activity" handler={UserActivity}/>
      <!-- add a route to handle anything that doesn't match details or actiivty -->
      <Router.Route path="user/*" handler={UserRedirect}/>
    </Router.Route>
  </Router.Route>
</Router.Routes>
var UserRedirect = React.createClass({
  statics: {
    willTransitionTo: function(transition, params) {
      transition.redirect('user-details', params);
    }
  }
});

(hopefully @mjackson's proposals in #142 don't ruin this solution)

@ryanflorence
Copy link
Member

I wonder if willTransitionTo should have a little more information. If you knew in User.willTransitionTo there that there is no activeRouteHandler you wouldn't need this extra route (though, I like having it, personally).

@ryanflorence ryanflorence changed the title Specifying default sub-routes Specifying default sub-routes and redirecting when there is no activeRouteHandler match Jul 30, 2014
@JedWatson
Copy link
Contributor Author

Thanks @rpflorence for the solution, two questions though:

  1. The 'catch-all' you've added should match user/:userId/* though, right? Would the trailing / also need to be optional or is that automatic?
  2. How does {User} know what the :userId is if it has no path defined? Does it receive params from child routes? (this would be neat, if it does)

The second was what led me to this question in the first place.

@ryanflorence
Copy link
Member

get rid of the name on the user route so it can't match if you don't want it to, and then probably do user* to not worry about the /.

@paulstatezny
Copy link

I've been doing:

componentWillMount : function() {
    if (! this.props.activeRouteHandler()) {
        Router.replaceWith(...);
    }
},

Didn't realize there was already a baked-in solution! Thanks @rpflorence.

@bobeagan
Copy link
Contributor

@rpflorence I still think that @JedWatson has some unanswered questions regarding how the handler will have knowledge of the :userId param if your path doesn't include it. A path of user* or a path of user/:userId* both seem like they would cause some (different) problems for what he is trying to do (redirect user/15 to user/15/details.

@JedWatson
Copy link
Contributor Author

@bobeagan is right - or alternatively, being able to specify a default subRouteHandler would solve the problem if that's not a crazy idea.

@ryanflorence
Copy link
Member

@JedWatson @bobeagan alright, here are scenarios we're solving as I understand them.

Scenario 1

  1. visitor is at /user/123/activity
  2. visitor gets curious and types in /user

You don't actually want to render anything. the route with user* will match, and then you redirect elsewhere in the app, and you can't go to the user page because you don't have the param.

Scenario 1

  1. visitor is at /user/123/activity
  2. visitor gets curious and types in /user/123

Our router config doesn't handle this yet, but lets add it:

<Routes>
  <Route handler={App}>
    <!-- remove path from this handler, don't want it to ever match a url -->
    <Route handler={User}/>
      <Route name="user-details" path="user/:userId/details" handler={UserDetails} />
      <Route name="user-activity" path="user/:userId/activity" handler={UserActivity}/>
      <!-- match user/123 -->
      <Route path="user/:userId" handler={UserRedirectToDetails}/>
      <!-- match everything else -->
      <Route path="user*" handler={UserRedirect}/>
    </Route>
  </Route>
</Routes>
# url -> handler
/user -> UserRedirect
/user/ -> UserRedirect
/user/123 -> UserRedirectToDetails
/user/lol -> UserRedirectToDetails (you will handle not finding a resource there)
/user/123/ -> I'm unsure, actually, this is why we want `?` in paths
/user/123/details -> UserDetails
/user/123/activity -> UserActivity

@ryanflorence
Copy link
Member

I think this would work for user/123 and user/123/

<Route path="user/:userId" handler={UserRedirectToDetails}/>
<Route path="user/:userId/" handler={UserRedirectToDetails}/>

@bobeagan
Copy link
Contributor

Verbos-ilicious! :)

@ryanflorence
Copy link
Member

yeah, we need ?. But so far it seems our current API can meet every use-case thats been thrown at it, some solutions less elegant than others, but the base is solid. Isn't React all about maybe writing a little more code, but knowing what on earth is going on?

For instance, if you jump into some code and see a route w/o the / you may wonder if matches or not, but if you see both routes you know "oh, thats dumb" but you also know how it works :P

@ryanflorence
Copy link
Member

(still, I'd like a more elegant solution)

@JedWatson
Copy link
Contributor Author

We've gotten caught up on the optional /... my real question here is:

If we remove the path from

<Route handler={User}/>

how does it know what the :userId parameter value is? that was the whole reason I was matching it like

<Route handler={User}  path="user/:userId"/ >

in the first place.

Everything else, we can work around.

@ryanflorence
Copy link
Member

if a route handler has no path, it only ever gets rendered because a child route path matched (or, the entire branch has no path and therefore the lowest child matches /)

So, if somebody types in user or user/, you have to send them somewhere else, you don't know enough to send them to anything that needs the :userid.

@ryanflorence
Copy link
Member

<Routes>
  <!-- this will only ever get rendered if... -->
  <Route handler={User}>
    <!-- ... one of these match -->
    <Route name="details" path="user/:userId/details" handler={Details}/>
    <Route name="activity" path="user/:userId/activity" handler={Activity}/>

    <!-- a default sub-route to render when the others don't match -->
    <Route name="user-default" path="user/:userId" handler={UserDefault}/>
  </Route>

  <!-- this doesn't need to exist, because ... -->
  <Route path="user" handler={NotFound}/>
  <!-- ... this will catch it -->
  <Route path="*" handler={NotFound}/>
</Routes>

edit: added user-default and names.

@JedWatson
Copy link
Contributor Author

I'm not coming across clearly, and I apologise if I'm missing something in React that makes this unnecessary / an anti-pattern.

Going with the whole "Nested UI" thing with Routes, my {User} handler wants to know the :userId param so it can do things with it.

It works if the following setup is in place:

  <Route path="user/:userId" handler={User}>
    <Route path="user/:userId/details" handler={Details}/>
    <Route path="user/:userId/activity" handler={Activity}/>
  </Route>

It seems I can:

  1. Go with the solution above and transition.redirect() from the {User} handler (in which case, I think the API should be improved because then I'm hard-coding the path in the condition
  2. Pass the :userId param back up to the {User} handler from the {Details} and {Activity} handlers
  3. Do something else..?

Maybe the solution is actually around <this.props.activeRouteHandler/>. Could that be replace with something like <this.props.activeRouteHandler || Details/>?

Hope this makes what I'm trying to solve clearer.

@ryanflorence
Copy link
Member

<Route handler={User}>
  <Route path="user/:userId/details" handler={Details}/>
  <Route path="user/:userId/activity" handler={Activity}/>
</Route>

Once upon a time the params were sent to User with this config, but it appears that is not the case anymore. Opening an issue:

So for now, you should use this route config:

<Route name="user" path="user/:userId" handler={User}>
  <Route name="user-details" path="user/:userId/details" handler={Details}/>
  <Route name="user-activity" path="user/:userId/activity" handler={Activity}/>
</Route>

And have your user handler do this:

var User = React.createClass({
  componentWillMount: function() {
    if (!this.props.activeRouteHandler())
      Router.replaceWith('user-details', this.props.params, this.props.query);
  }
});

@mjackson
Copy link
Member

mjackson commented Aug 1, 2014

@JedWatson Please see my comments in #158. I'm curious to know what User needs to do with userId. Redirect is most likely, and that case is already solved.

@JedWatson
Copy link
Contributor Author

Hey guys, sorry for the delay coming back to this.

@rpflorence that code for componentWillMount in User is exactly what I was missing. Thanks!

Will pick this up over in #158

@d4nyll
Copy link

d4nyll commented Jun 14, 2016

For those coming in 2016, you'd use <Indexredirect>

<Route path="/" component={App}>
  <IndexRedirect to="/welcome" />
  <Route path="welcome" component={Welcome} />
  <Route path="about" component={About} />
</Route>

@remix-run remix-run deleted a comment from yoiang Oct 16, 2017
@pratikt-cuelogic

This comment has been minimized.

@remix-run remix-run locked and limited conversation to collaborators May 14, 2018
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

7 participants