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

Support <Redirect> in <Switch> #4541

Closed
wmertens opened this issue Feb 17, 2017 · 14 comments
Closed

Support <Redirect> in <Switch> #4541

wmertens opened this issue Feb 17, 2017 · 14 comments
Labels

Comments

@wmertens
Copy link
Contributor

wmertens commented Feb 17, 2017

Right now, to implement a redirect fallback in a switch, you need to do

<Switch>
  <Route path="/foo" component={Bar}/>
  <Route path="/bar" component={Baz}/>
  <Route render={() => <Redirect to="/foo"/>}/>
</Switch>

How about checking for the to parameter in the Switch loop and matching directly:

<Switch>
  <Route path="/foo" component={Bar}/>
  <Route path="/bar" component={Baz}/>
  <Redirect to="/foo"/>
</Switch>
@timdorr timdorr changed the title [v4] should <Switch> support <Redirect>? Should <Switch> support <Redirect>? Feb 17, 2017
@timdorr
Copy link
Member

timdorr commented Feb 17, 2017

An interesting idea. However, I'm not sure it would have the intended consequence. To me, it looks like the Redirect is either misplaced or will always be performed, regardless of the Switch's match. The Route with a render prop is more verbose, but definitely clearer about what's going to happen here.

@wmertens
Copy link
Contributor Author

So, to me, it's harder to see at a glance that the last <Route/> does not have a path and will always match. I read the second version as "Oh, a switch, that only renders the first match... looks like always matches... ah so it's a fallback"

@timdorr
Copy link
Member

timdorr commented Feb 17, 2017

You could do this if the undefined path is not clear:

const createRedirect = to => () => <Redirect to={to} />
// ...
<Route path="/*" component={createRedirect('/foo')} />

@wmertens
Copy link
Contributor Author

A bit sugary to my taste, with runtime cycles and memory going to devel time enjoyment…

@mjackson
Copy link
Member

I like it. Maybe in this case <Redirect> could also take a from prop (like we did in previous versions) so you could use a few of them in a <Switch>.

<Switch>
  <Route exact path="/home" .../>

  // Simple <Redirect>
  <Route path="/about" .../>
  <Redirect from="/company" to="/about"/>

  // <Redirect> with dynamic URL segments
  <Route path="/users/:userId" .../>
  <Redirect from="/profile/:userId" to="/users/:userId"/>

  // A <Redirect> without a "from" prop always matches. Same as a <Route> with no "path".
  <Redirect to="/home"/>
</Switch>

@mjackson mjackson changed the title Should <Switch> support <Redirect>? Support <Redirect> in <Switch> Feb 17, 2017
wmertens added a commit to wmertens/react-router that referenced this issue Feb 17, 2017
* `<Redirect>` already worked because it doesn't have a `path` and it gets rendered :)
* Add `from` as an alias for `path` in `matchPath` so you can write `<Redirect from="/old" to="/new"/>`

Fixes remix-run#4541
@wmertens
Copy link
Contributor Author

@mjackson turns out that was remarkably easy to do - just make from an alias for path

mjackson pushed a commit that referenced this issue Feb 17, 2017
* `<Redirect>` already worked because it doesn't have a `path` and it gets rendered :)
* Add `from` as an alias for `path` in `matchPath` so you can write `<Redirect from="/old" to="/new"/>`

Fixes #4541
@mjackson
Copy link
Member

@wmertens Feels like validation that we're on the right track! :D

@wmertens
Copy link
Contributor Author

wmertens commented Feb 17, 2017

@wmertens Feels like validation that we're on the right track! :D

Definitely, I love the new API! I was pretty annoyed with the previous RR versions, but now, I'm a fan! 💯 (especially with the upcoming relative path support, leading to true componentization)

@hamzakubba
Copy link

What do you think of:

<Switch>
  <Route exact path="/home" .../>

  <Route path="/about" .../>
  // redirectTo prop has lower precedence than component and render props
  <Route path="/company" redirectTo="/about"/>

  <Route path="/users/:userId" .../>
  <Route path="/profile/:userId" redirectTo="/users/:userId" redirectPush/> // redirectPush boolean
  <Route path="/profile/:userId" redirectPushTo="/users/:userId" /> // another way to do push

  // always matches
  <Route redirectTo="/home"/>
</Switch>

PS: What's the use-case for redirect with push?

@pshrmn
Copy link
Contributor

pshrmn commented Feb 18, 2017

@hamzakubba <Redirect push> pushes a new location onto the the history stack whereas the default <Redirect> replaces the current location.

@hamzakubba
Copy link

hamzakubba commented Feb 18, 2017

@pshrmn I know, I meant why would anyone want to do that? I was curious why it was implemented on the Redirect component in the first place.

@pshrmn
Copy link
Contributor

pshrmn commented Feb 18, 2017

#3903 appears to be the origin of that, so I assume that there are use cases listed there (just skimmed, didn't really look that close).

@mjackson
Copy link
Member

@hamzakubba I like the <Redirect> component better than overloading <Route>'s props.

@georgiosd
Copy link

Is this supported now? I thought it was but couldn't get it to work - went with <Route component={NoMatch} /> isntead

@lock lock bot locked as resolved and limited conversation to collaborators Jan 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants