Hi,
I just watched your talk about the v4 api and overall I love the new component-centric interface, except for the Redirect component, which is an abomination.
If there is a discussion on this outside of twitter, I have not been able to find it, so I thought it might be a good Idea to open an issue for that purpose.
Anyway, the whole side-effect-as-component idea violates pretty much every intuition I have about how render in react should work.
One of the things that really stuck with me after re-watching Pete Hunts original talk about rethinking best practices was how the first time I rolled my eyes every time he said how great it was that React re-renders everything on every change, which seemed like such a waste, and later I thought this was a genius ploy to make people write more functional code by telling them it will be called over and over again at times they cannot control.
_Just return data, avoid side effects._
This matches how setState() should be used in render, right? You can wire up callbacks that cause changes later, but you shouldn't call setState while render is running. People have gotten into infinite loops by doing that.
_Render now, change later._
What would this even mean:
<hr>
<Redirect ...>
<hr>
<Redirect ...>
Render a little bit, then interrupt the rendering, throw what has been rendered away, redirect, re-render from scratch and what about the components that came after the Redirect? It just... no.
As far as I can tell, there is only one place where Redirect should be used, which is directly inside of Match, like in this example: https://react-router.now.sh/auth-workflow
I believe the Redirect-component should be slain and replaced with a custom callback on Match, so the example would become something like:
const MatchWhenAuthorized = ({ component: Component, ...rest }) => (
<Match {...rest} testBefore={ (props, redirect) => (
if (fakeAuth.isAuthenticated) {
return true
} else {
redirect({
pathname: '/login',
state: { from: props.location }
})
return false
}
)}
render={ props => (
<Component {...props}/>
)}
/>
)
With this approach, everything fits neatly into my mental model of _render now, change later_, because Match does not define a side-effect, it defines a listener.
It does not try to abort the rendering process. There is no problem with what it means to have Match before or after other components or what it means to have several Matches.
There are a number of things I don't know, like what the callback should be named, what parameters exactly it should be passed, or if it is necessary to return a bool to indicate whether the match should render. If calling the redirect-callback reliably prevented the match from rendering, the return value could be omitted.
Also, for controlling code like flux or redux, whatever path the ControlledRouter uses should be paved as the only other legitimate place for redirects that I can see.
https://github.com/ReactTraining/react-router/issues/3879
The name might be wrong. Maybe consider Redirect as <Address location/>
instead, and its re-renders are idempotent, just like a <input value/>
. Not so weird now?
You can wrap match and create your own MatchWithTestBefore
, access router
from context and see how you like it. Let us know :)
Anyway, the whole side-effect-as-component idea violates pretty much every intuition I have about how render in react should work.
Here's another way to think about it, Redirect renders to browser's location bar. Every render is a side-effect, whether it's text in an input or text in the address bar.
How can <Redirect/>
be idempotent, unless it redirects to the match it is under right now?
While every render may be a side-effect, there is set of expectations that makes renders well-behaved. <input value/>
can be added and removed, you can have several of them and they never abort the rendering process.
There is a huge difference between the local effect of inserting an input element into the DOM and the non-local effect of scrapping the rendering process to render something else entirely.
Side-effects don't happen in render, they are care taken off by the reconciler that figures out if side-effects are necessary. Render just returns data. That is the magic of react and <Redirect/>
breaks that by forcing an immediate non-local effect.
Most helpful comment
How can
<Redirect/>
be idempotent, unless it redirects to the match it is under right now?While every render may be a side-effect, there is set of expectations that makes renders well-behaved.
<input value/>
can be added and removed, you can have several of them and they never abort the rendering process.There is a huge difference between the local effect of inserting an input element into the DOM and the non-local effect of scrapping the rendering process to render something else entirely.
Side-effects don't happen in render, they are care taken off by the reconciler that figures out if side-effects are necessary. Render just returns data. That is the magic of react and
<Redirect/>
breaks that by forcing an immediate non-local effect.