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

Redirections don't trigger rendering #360

Closed
baskerville opened this issue Oct 6, 2014 · 17 comments · Fixed by #384
Closed

Redirections don't trigger rendering #360

baskerville opened this issue Oct 6, 2014 · 17 comments · Fixed by #384

Comments

@baskerville
Copy link

Please consider the following example:

var React = require("react"),
    Router = require("react-router"),
    Routes = Router.Routes,
    Route = Router.Route,
    DefaultRoute = Router.DefaultRoute;

var ForbiddenRoute = {
    statics: {
        willTransitionTo: function (transition) {
            transition.redirect("/");
        }
    }
};

var Home = React.createClass({
    render: function () {
        return (
            <p>Home</p>
        );
    }
});

var GoHome = React.createClass({
    mixins: [ForbiddenRoute],
    render: function () {
        return null;
    }
});

var App = React.createClass({
    render: function () {
        return (
            <this.props.activeRouteHandler/>
        );
    }
});

var routes =  (
    <Routes location="history">
        <Route path="/" handler={App}>
            <Route path="/gohome" handler={GoHome}/>
            <DefaultRoute handler={Home}/>
        </Route>
    </Routes>
);

React.renderComponent(routes, document.body);

After visiting /gohome I would expect Home to be rendered, but it isn't.

@licx
Copy link

licx commented Oct 7, 2014

I'm experiencing the same issue

@mjackson
Copy link
Member

mjackson commented Oct 7, 2014

What happens if your statics are defined in GoHome rather than in your ForbiddenRoute mixin?

@vimto
Copy link

vimto commented Oct 8, 2014

I'm experiencing this too, with the equivalent of the statics defined in the component rather than in a mixin.

@baskerville
Copy link
Author

@mjackson
The result is the same.

@vimto
Copy link

vimto commented Oct 8, 2014

FYI, same result when I try and use the <Redirect /> component.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2014

@baskerville Can you please try this on 0.9.2? We shipped a fix yesterday that should address this problem.

@baskerville
Copy link
Author

The issue still holds with 0.9.2.

@dancannon
Copy link
Contributor

I am also experiencing this but only when location="history". Here is the code I use to redirect:

/** @jsx React.DOM */
var React = require('react');
var {Navigation} = require('react-router');

var toolkitApi = require('../toolkit/api');
var authUtils = require('../utils/auth');

module.exports = React.createClass({
    displayName: 'AuthenticatedContainer',

    mixins: [Navigation],

    statics: {
        willTransitionTo: function(transition, params) {
            if (!authUtils.isAuthenticated()) {
                return transition.redirect('login', params, {
                    redirect: transition.path
                });
            }
        }
    },

    onLogout: function() {
        this.transitionTo('/', this.props.params);
    },

    componentWillMount: function() {
        toolkitApi.on('logout', this.onLogout);
    },

    componentWillUnmount: function() {
        toolkitApi.off('logout', this.onLogout);
    },

    render: function() {
        return this.props.activeRouteHandler();
    }
});

@baskerville
Copy link
Author

I can confirm that the issue isn't happening when location="hash".

@ryanflorence
Copy link
Member

The only time the redirect doesn't render is when the page is initially loaded at the route that redirects. If you navigate to it in an already loaded app it works as expected.

@ryanflorence
Copy link
Member

git bisect tells me 47f0599 with all of its awesomeness introduced this. @Gearon, any knee jerk reactions to what might have caused this happen?

@mjackson
Copy link
Member

@gaearon ^^^

@gaearon
Copy link
Contributor

gaearon commented Oct 10, 2014

Is this unrelated to #336?

@gaearon
Copy link
Contributor

gaearon commented Oct 10, 2014

I may be misreading the issue but at a glance it looks exactly like #336 which was introduced a bit earlier than my PR, in c96e34d.

However, you said it was fixed in 0.8.0. Could something from 0.9.x break it again?

@roycehaynes
Copy link
Contributor

Appreciate the contributors to this project! Experiencing exact same issue mentioned here on v0.9.3. What was the resolution?

@roycehaynes
Copy link
Contributor

I just added the fix to node_modules temporarily until this is updated in npm. Thanks guys!

@ryanflorence
Copy link
Member

there will be a new release on monday, if not sooner.

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

Successfully merging a pull request may close this issue.

8 participants