-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Redirect component kills infinite redirect loop (good thing) which masks the issue (bad thing) #5003
Comments
Alright, just for anyone else that reads this, this is the issue:
What this means is that if the I'm not really sure that a fix is necessary, but if we were to address it, the easiest approach would probably be to add a componentDidUpdate() {
warning(false, 'A <Redirect> should never update.')
} |
Hello, could I take this on as a beginner commit? |
No need to ask, just submit a PR 😄 |
@Dustin-RW @pshrmn The case of redirecting to the same route should definitely warn, blank pages are a frustrating dev experience (just had it and spent 2 hours getting to a similar conclusion as @pshrmn before reading this issue). But... there's another case... See Root.js:
The redirects will actually change the URL, but since Switch creates a clone of the child (Redirect in this case), and the previous child was also a Redirect, there's no logic on props change, and Redirect renders So in
Questions:
@Dustin-RW Are you on it, or do you want me to handle it? |
@pshrmn I'd like to take this, can you answer my questions in the comment above? |
As a temporary fix, you could set a <Switch>
<Redirect
exact
from="/"
to="/hello"
key="from-root"
/>
<Redirect
exact
from="/hello"
to="/hello/world"
key="from-hello"
/>
<Route exact path="/hello/world" component={ Hello } />
</Switch> |
@pshrmn Cool, thanks. |
Is there any movement on this PR? It looks to have been hanging for almost 3 weeks. Having an issue #5231 and hoping that this PR with using Keys will be able to solve the problem. Having extreme difficulties getting clarity from multiple channels of communication so this would be nice to try. |
@wsfuller Done #5162 (comment) :-) Note that adding |
@alexilyaev Thank you so much! Been really struggling with this problem for the past several days pulling out hair |
Version
4.0.0
Test Case
https://codepen.io/anon/pen/dWGwwY
Steps to reproduce
Code pen example has this issue shown when visiting About page twice!. Click About > Home > About.
Have some logic in your app that redirects the page if a certain parameter isn't set, or set wrong. Upon visiting the page a second time you might step into that redirect unintentionally. This is a simple logic error caused by the developer, fair enough. However, the Redirect component seems to swallow this without notifying the developer, causing great pain and anguish in trying to track down how a page is now blank
Expected Behavior
Maybe a console log in ENV=development that 'Redirect did not cause a change to URL' or something like that?
Actual Behavior
Pain.
Anguish.
(both great)
The text was updated successfully, but these errors were encountered: