Suggestions on integration with react-router (Morearty.js)

[Tvaroh]

Hello.

I faced a problem of integrating Om-style library (Morearty) with react-router. Problems are:

• rendering should be managed by the library itself, not by the router;
• when defining routes, components with some properties pre-set should be passed, like <Route name="about" handler={ About({ binding: <...> }) } />.

So, as I understand, awesome react-router should just play two roles:

• notify when route is changed;
• allow to get a component which should be displayed currently.

This should be enough to embed the router. Can you provide some feedback if it's possible to use react-router this way?

[klimashkin]

+1

[ArnoBuschmann]

+1

A working react-router-morearty integration would be awesome.

[gaearon]

I think this is def possible with new API. Router now just calls you back and it's up to you to do the rendering and pass props.

[benoneal]

@gaearon could you please give a brief code example of how react-router could be set up to pass the current route state object into Morearty?

[mjackson]

@Tvaroh Essentially you need to listen for two things:

1. changes in the route
2. changes in your binding

The router API is optimized for the first use case, but it should be really easy to make it work with the second as well. I haven't used Morearty before (but it looks awesome! def going to look further into it), so please forgive me if the syntax is off.

// This is a reference to my Morearty binding object.
var binding = ...;

// Keep a reference to the "current handler", which is the React component
// class that is at the root of your route hierarchy.
var CurrentHandler;

function renderTheCurrentHandler() {
  React.render(<CurrentHandler binding={binding}/>, document.body);
}

binding.addListener(function () {
  // This is my notification that something in the binding changed.
  renderTheCurrentHandler();
});

Router.run(routes, function (Handler) {
  // This is my notification that the route changed.
  CurrentHandler = Handler;
  renderTheCurrentHandler();
});

Does something like that work for you?

[Tvaroh]

@mjackson, the problem I'm facing is type error thrown from getChildContext method in RouteHandler.js since this.context.routeHandlers is undefined on first render. As I understand, this is because rendering is performed by Morearty - not from callback passed to Router.run. This is by design, Morearty knows when something has changed in app state and triggers re-rendering. Probably this is the reason why react-router context isn't properly initialized.

I guess, if the first render succeeds, subsequent render will return same components from route handlers, is it true? If so, it would be sufficient to ask Morearty to re-render in react-router route callback.

To summarize:

• Since Morearty should render the app itself, is it possible to pre-initialize react-router so that RouteHandlers would return something meaningful?
• When a route changes, react-router just changes route handlers (if I understand correctly), so in route callback one asks Morearty to re-render.

Could you suggest something?

[gaearon]

@Tvaroh

Can you just disable Morearty render callback until router okays it?

[Tvaroh]

I tried

var firstRender = true;
var Bootstrap = Ctx.bootstrap(App);

Router.run(routes, function (Handler) {
  if (firstRender) {
    React.render(<Bootstrap />, document.body);
    firstRender = false;
  } else {
    Ctx.queueFullUpdate();
    b.meta().set('dummy'); // ask to re-render
  }
});

but still having the same error. Maybe this is somehow related to the fact I ignore Handler argument.

[gaearon]

Yeah it is related. Handler is what owns the context. Why do you not want to use it?

[Tvaroh]

But how can I? React.render(bootstrappedRootComponent, target) is called only once in Morearty and then all re-rendering is triggered internally using forceUpdate. I don't need the Handler argument here. I was hoping that, when route changes, RouteHandlers just return some other components.

[gaearon]

Can you just make Ctx.bootstrap result your root route handler in routes?

https://github.com/moreartyjs/moreartyjs#starting-the-application

Then let react-router do top level rendering with Handler.

[Tvaroh]

Yep, unfortunately still having the same error.

var routes = (
  <Route name="app" path="/" handler={Ctx.bootstrap(App)}>
    <DefaultRoute handler={Dashboard} />
    <Route name="inbox" handler={Inbox} />
    <Route name="calendar" handler={Calendar}>
      <Route name="foo" handler={Foo} />
    </Route>
  </Route>
);

Router.run(routes, function (Handler) {
  React.render(<Handler />, document.body);
});

[gaearon]

It shouldn't be a problem, I render my app in rAF and it works perfectly with RR.
Can you share a minimal runnable reproducible example? I can try to make it work.

[Tvaroh]

Yep, I've removed RAF part from the comment after realizing that the first blocking render isn't performed at all.

[Tvaroh]

Runnable example: https://github.com/moreartyjs/todomvc-moreartyjs/tree/react-router

[gaearon]

Thanks for TodoMVC link. The issue here is Morearty using React.withContext which is btw getting deprecated soon. This context overrides context set by router.

I see they offer to pass custom context to bootstrap but we can't seem to do that because we don't know context before we run the router!

This is solvable, just needs more thought. There's probably some simple workaround.

[Tvaroh]

Can you suggest an alternative to withContext?

[gaearon]

Yes, Morearty can wrap its root component into a dynamically generated React class which would provide context via getChildContext. Just like react-router does it.

[Tvaroh]

Awesome, thank you. Will try to migrate on this approach.

[gaearon]

So there are two ways you can solve this:

1. Wrap RR into a React component that would live "under" Morearty and be the one you call bootstrap on. You would run router in componentWillMount, react to its callback by doing setState({ handler: Handler }) and return handler from render.
2. Better, fix Morearty to avoid using withContext in favor of getChildContext.

[Tvaroh]

Yep, I think getChildContext is the way to go.

[gaearon]

Now that I tried it, seems like RR doesn't pass outer context down so this won't work either.
(And it can't because it would need to know about Morearty in order to receive this context).

I'm not so sure how to do it now..

[gaearon]

This is a bit dirty but it works for me:

var App = React.createClass({
  mixins: [Morearty.Mixin],
  render: function () {
    return (
      <div>
        <h1>App</h1>
        <RouteHandler />
      </div>
    );
  }
});

var RouterMoreartyWrapper = React.createClass({
  contextTypes: {
    getRouteAtDepth: React.PropTypes.func.isRequired,
    getRouteComponents: React.PropTypes.func.isRequired,
    routeHandlers: React.PropTypes.array.isRequired
  },

  render: function () {
    var RouteHandlerWithContext = Ctx.bootstrap(RouteHandler, this.context);
    return <RouteHandlerWithContext />;
  }
});

var routes = (
  <Route handler={RouterMoreartyWrapper}>
    <Route name="app" path="/" handler={App}>
      <DefaultRoute handler={Dashboard} />
      <Route name="inbox" handler={Inbox} />
      <Route name="calendar" handler={Calendar}>
        <Route name="foo" handler={Foo} />
      </Route>
    </Route>
  </Route>
);

Router.run(routes, Router.HistoryLocation, function (Handler) {
  React.render(<Handler />, document.body);
});

[gaearon]

This is not all context though. You'll need to add childContextTypes of all mixins that router uses, to the Wrapper.childContextTypes. See router's mixins:

https://github.com/rackt/react-router/tree/master/modules/mixins

[gaearon]

This is actually an interesting problem here. Say we have Router and Lib X. Both provide child context.

If Lib X doesn't offer a way to specify custom context like Morearty does, we can't use it with Router because there is just no way we can pass that context down. For if Lib X is inside Router, handlers won't get Router's context. If Router is inside Lib X, handlers won't get Lib X's context.

I guess it's still an issue Lib X should solve because Router makes more sense at the top.

[Tvaroh]

Having some problems migrating from withContext. For a reason even with current approach if I change

contextTypes: { morearty: function () {} } // this is the only way it works

to

contextTypes: { morearty: React.PropTypes.instanceOf(Context).isRequired }
// or
contextTypes: { morearty: React.PropTypes.object.isRequired }
// or even
contextTypes: { morearty: React.PropTypes.any.isRequired }

I get Warning: Required context morearty was not specified in <<anonymous>>.

So, when I remove withContext and migrate to childContextTypes / getChildContext (exactly like here), I see the warning above as well as undefined this.context.morearty.

[gaearon]

@Tvaroh I'm a bit confused. Are you saying approach from this comment isn't working? I think it ran on my computer.

[Tvaroh]

@gaearon, no, I just tried to migrate from withContext to newer API but with no luck.

[gaearon]

@Tvaroh

I just tried to migrate from withContext to newer API but with no luck.

Oh. I thought we were talking about making it work with withContext-based API that Morearty had at the time I wrote that comment. My hackish example above should work with that API. Do you still want to give it a try?

---

If you want to make withContext-less API work, do you have a separate runnable example where I can see what went wrong?

[Tvaroh]

Yep, I understand that this should work with withContext as well. Was just wondering why this React context stuff didn't work as expected.

Your example above worked partially but since I had no time to debug it further. Will reply here when I'll have something to share.

[gaearon]

Yep, I understand that this should work with withContext as well. Was just wondering why this React context stuff didn't work as expected.

Can you show the complete changes? I suspect childContextTypes are not declared, or something like that.

[Tvaroh]

I've committed something to the repo above (continuing your example). For a reason wrapper's render is called only once on init. Additionally, if you load app having inbox section in the URL then switching to other sections invokes route handler (it's visible in console log), but switching back to the initial section doesn't call the handler. Also nothing is re-rendered after initial render but I see rootComp.forceUpdate() is called within Morearty (but has no effect for a reason).

[Tvaroh]

Gave you write permissions to the repo for convenience.

[gaearon]

I'll take a look. Thanks!

[Tvaroh]

Can asynchronous rendering in handler callback cause problems? It only queues render but the rendering itself is performed in rAF later.

[gaearon]

I have an RR powered app with rAF batching and it works fine. Shouldn't be an issue.

[mjackson]

If Lib X doesn't offer a way to specify custom context like Morearty does, we can't use it with Router because there is just no way we can pass that context down. For if Lib X is inside Router, handlers won't get Router's context. If Router is inside Lib X, handlers won't get Lib X's context.

@gaearon isn't the return value of getChildContext simply added to any context that comes down from parent components? i.e. if the router provides a child context, but the router is already running in some other context, shouldn't children be able to access context from either one based on its contextTypes?

[gaearon]

I thought so but it did not work for me that way. Perhaps I made some other mistake though. Need to check again!

[ryanflorence]

Can't we just do the router stuff in the top level morearty component, sorta like this? #579 (comment)

[ryanflorence]

This works: http://jsbin.com/duyaxa/4/edit?html,js,output

Storing Handler in this.state is obviously not something Morearty likes, but I don't know how to put it into Ctx.

Let me know if I should reopen.

[honzatrtik]

@ryanflorence unfortunately this wont work on server - componentDidMount is never invoked there.

[ryanflorence]

The trick is to realize "everything is possible with React"

Here's your server rendering code 💃

http://jsbin.com/duyaxa/8/edit?js,console

[honzatrtik]

Yes, you are correct. Even if this solution feels little bit hacky to me, it works. Thank you very much @ryanflorence.

[ryanflorence]

going to be hacks when two libs want to own the entry of your app :), fortunately ours is more flexible whereas morearty completely insists on being "first".