Router: Regression: Redirect behavior broken in 1.3.1

Created on 4 Feb 2020  路  6Comments  路  Source: reach/router

If you have a router like:

<Router primary={false}>
    <ClusterList path="/"/>
    <ClusterPage path="cluster/:cluster/*"/>
    <Redirect from="cluster/:cluster" to="cluster/:cluster/pods" noThrow={true}/>
</Router>

and you have a link like:

<Link to={`/cluster/mycluster`}>mycluster</Link>

the resulting url will read like:

http://localhost:8082/cluster/mycluster/cluster/mycluster/pods

Notice the repeating slugs.

This wasn't present in 1.2.x.

Most helpful comment

A similar comment was made here:
https://github.com/reach/router/compare/v1.3.0...master

I actually like the change, I was wondering why it did not work this way before. It is unfortunate that it was released as a breaking change.

I use <Redirect/> inside nested routers because using a default= does not actually change the window location. It was very annoying to have to include the full path in a Redirect because it defeated the idea of a nested router since it would always have some awareness of its location.

In these examples, I want /somewhere to go to /somewhere/else and render <Something/>

Before:

<RouterOne>
  <RouterTwo path='somewhere'>
   <Redirect
      path='/'
      to='/somewhere/else' // see the extra usage of "somewhere"
    />
   <Something path='else' />
  </RouterTwo>
</RouterOne>

Now:

<RouterOne>
  <RouterTwo path='somewhere'>
   <Redirect
      path='/'
      to='/else' // works in any router, not just under /somewhere
    />
   <Something path='else' />
  </RouterTwo>
</RouterOne>

(also notice that path can replace from in <Redirect/>)
Edit: nevermind path, I export my own wrapped Redirect.

All 6 comments

A similar comment was made here:
https://github.com/reach/router/compare/v1.3.0...master

I actually like the change, I was wondering why it did not work this way before. It is unfortunate that it was released as a breaking change.

I use <Redirect/> inside nested routers because using a default= does not actually change the window location. It was very annoying to have to include the full path in a Redirect because it defeated the idea of a nested router since it would always have some awareness of its location.

In these examples, I want /somewhere to go to /somewhere/else and render <Something/>

Before:

<RouterOne>
  <RouterTwo path='somewhere'>
   <Redirect
      path='/'
      to='/somewhere/else' // see the extra usage of "somewhere"
    />
   <Something path='else' />
  </RouterTwo>
</RouterOne>

Now:

<RouterOne>
  <RouterTwo path='somewhere'>
   <Redirect
      path='/'
      to='/else' // works in any router, not just under /somewhere
    />
   <Something path='else' />
  </RouterTwo>
</RouterOne>

(also notice that path can replace from in <Redirect/>)
Edit: nevermind path, I export my own wrapped Redirect.

@jsphstls

Yes, I must admit that doing absolute links in nested elements was inconvenient 馃憤
However, now with new behaviour I ran into some specific issue.

For sake of simplicity, let's say I want to create a page that will list inactive users in my platform. So, I create a route /users/inactive. If someone hits just /users I want him to be redirected to /users/inactive by default. So, I create my router:

<Router>
   <UserComponent path="users">
    <Redirect from="/" to="inactive" />
    <UserInactiveComponent path="inactive" />
  </UserComponent>
</Router>

Works just fine.
Now I want to make it a little bit more dynamic and instead of making separate components for each user status I want to create a dynamic one that consumes parameters. I want to add pagination as well.
So, after the changes my router looks like:

<Router>
   <UserComponent path="users">
    <Redirect from="/" to="inactive/1" />
    <UserByStatusComponent path=":status/:pageNr" />
  </UserComponent>
</Router>

So far, so good. If someone hits just users then he gets redirected to users/inactive/1. But what if someone hits users/inactive and I want to redirect him to page 1? I would need to create another redirection rule:

<Router>
   <UserComponent path="users">
    <Redirect from="/" to="inactive/1" />
    <Redirect from=":status" to=":status/1" /> // <----- new rule
    <UserByStatusComponent path=":status/:pageNr" />
  </UserComponent>
</Router>

Boom! 馃挜 This rule redirects from users/inactive to users/inactive/inactive/1 and since from is based on parameter it cannot be omitted in to rule because

Uncaught Invariant Violation: <Redirect from=":status" to="1"/> has mismatched dynamic segments, ensure both paths have the exact same dynamic segments.

Did I some logical mistake here or new Redirect behaviour doesn't go well with parameters?

@grezegorzezerz what happens if you use path instead of from on the Redirects?
And what about path="/:status" ?

Still, there is no question that this an issue because a regression was introduced.

@jsphstls Thank you for a quick reply 馃憤
I've tried it, but unfortunately from is required in Redirect element:

Uncaught Invariant Violation: <Redirect from="undefined" to=":type/page/1"/> requires both "from" and "to" props when inside a <Router>.

I think for now I'll try to implement something custom using either default property or Match element. 馃槂

My bad, I forgot that I export my own Redirect as a wrapper of this Redirect. Getting lost in my own code :sweat_smile:

maybe we face the same problem, eg: https://github.com/reach/router/issues/389

Was this page helpful?
0 / 5 - 0 ratings

Related issues

danoc picture danoc  路  3Comments

kbrgl picture kbrgl  路  4Comments

ssured picture ssured  路  3Comments

magnusarinell picture magnusarinell  路  3Comments

sseppola picture sseppola  路  4Comments