martyjs / marty

A Javascript library for state management in React applications
http://martyjs.org
MIT License
1.09k stars 77 forks source link

support statics in marty containers to support react-router transitions #322

Closed TN1ck closed 9 years ago

TN1ck commented 9 years ago

I think this is a common procedure to achieve authorized content in a react-application:

In marty you would try to implement this with a marty-container that calls a check_login method in the fetch method. Problem is, it is impossible to call the transitionTo or replaceWith of react-router to make a server-side redirect. You'll always get the error Failed to render /private Error: Invariant Violation: You cannot modify a static location.

Here is it how you would do with a react-router-mixin, this will not get executed when the component is inside a marty-container.

var requireAuth = {
    statics: {
        willTransitionTo: function (transition, params, query, callback) {
            user.isLoggedIn((result) => {
                if (!result) {
                    transition.redirect('/login',
                        {},
                        {'nextPath' : transition.path});
                }
                callback();
            });
        }
    }
};

Here is how I tried it with marty:

var Private = React.createClass({
    render: function () {
        return (
            <div>
                <Header />
                <div className="content">
                    <RouteHandler />
                </div>
            </div>
        );
    }
});

module.exports = Marty.createContainer(Private, {
    contextTypes: {
        router: React.PropTypes.func.isRequired
    },
    fetch () {
        return {
            user: UserStore.for(this).isLoggedIn()
        };
    },
    pending () {
        return <div></div>;
    },
    failed () {
        var router = this.context.router;
        router.replaceWith('/login',
            {},
            {'nextPath' : router.getCurrentPath()});
        return <div></div>;
    }
});
idolize commented 9 years ago

@TN1ck Given that many other Flux implementations have been moving away from singletons, this issue has actually been known for some time in the react-router community. There is a very informative thread here where it is discussed.

The gist of it is this: it looks like react-router 1.0 will address this problem.

Until then, however, you can just make a singleton version of your Marty.Application subclass (this works as long as you aren't using isomorphism):

// "app.js"
import Marty from 'marty';

// TODO move outside of singleton once React-Router 1.0 ships
class Application extends Marty.Application {
  constructor(options) {
    super(options);
    // etc...
  }
}

// Mimic the singleton pattern of the old Marty.register
const app = new Application();
export default app;

If you are using isomorphism, you are out of luck right now unless you want to use the fork of react-router that I linked in that pull request above.


With that said, if there are other use cases of how Marty containers would work with static app instances (I think that's what you are asking) then it is still worth mentioning here.

TN1ck commented 9 years ago

Thank you for the detailed answer. Yes the main problem was about isomorphism. I think i'll just wait for react-router to reach 1.0 and marty-express supports it.

I thought this is a marty-related problem, this issue can be closed then.

taion commented 9 years ago

Tangentially related to #338, but not really.

I agree that this is more of an issue with the react-router API combined with the use of context to store the app.

One alternative is to use something like continuation-locals for isomorphism, e.g.: https://github.com/taion/cls-isoflux-marty/blob/master/examples/httpbin-echo/server.js.