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

Integration with relay-tools/react-router-relay. #21

Open
joonhocho opened this issue Jun 14, 2016 · 5 comments
Open

Integration with relay-tools/react-router-relay. #21

joonhocho opened this issue Jun 14, 2016 · 5 comments

Comments

@joonhocho
Copy link

I like how this project reuses react-router, and I think this is the right direction for true code sharing between native apps and web.
I want to open an issue for integration with react-router-relay as well.
I'm not sure if it would work out of box as is now, but I will play with it soon to see whether it needs some modifications.

@jmurzy
Copy link
Owner

jmurzy commented Jun 15, 2016

This would be an awesome addition. If you'd like to lead this effort, it would definitely be welcome! Feel free to get in touch on Discord.

At first sight it looks like react-router-relay is just a router middleware router middleware, which seems to import the default RouterContext directly, ☹️, from React Router. RouterContext is our entry point to React Router. So as a first step we would have to submit a patch to ryanflorence/react-router-apply-middleware applyRouterMiddleware to make this work:

applyMiddleware = (RouterContext) => (...middleware) => ();

🍺

@joonhocho
Copy link
Author

joonhocho commented Jun 15, 2016

Correct me if I am wrong, but it seems the react-router-relay uses this applyRouterMiddleware.

It matches the signature as it provides renderRouterContext and renderRouteComponent.

I don't think there is any dependency to ryanflorence/react-router-apply-middleware.

@joonhocho
Copy link
Author

joonhocho commented Jun 15, 2016

I wonder if react-router-native can be refactored so that itself is also a middleware to react-router.

If done well, the final code with relay will look like the following:

import Relay from 'react-relay';
import useRelay from 'react-router-relay';
import {
  useNative,
  nativeHistory,
  TabsRoute
} from 'react-router-native';
import {
  Router,
  Route,
  applyRouterMiddleware
} from 'react-router';

/* ... */

const AboutQueries = {
  about: () => Relay.QL`query { about }`
};

const router = (
  <Router
    history={nativeHistory}
    render={applyRouterMiddleware(useNative, useRelay)}
    environment={Relay.Store}
    addressBar
  >
    <TabsRoute
      path="app"
      component={App}
      transition="horizontal-pager"
    >
      <Route
        path="/"
        component={About}
        queries={AboutQueries}
        overlayComponent={AboutHeader}
      />
    </TabsRoute>
  </Router>
);

@jmurzy
Copy link
Owner

jmurzy commented Jun 15, 2016

You're right. I wasn't aware of this—looks like that was merged to master as of v2.3.0.

Either way, we would have to have full control over RouterContext to be able to support rendering with NavigationExperimental. So a middleware approach won't work.

I just created a PR that adds applyRouterMiddleware that uses the "native" RouterContext. I don't think I should merge it until we have proper testing in place. Can you play with that branch and react-router-relay, and let us know if it works at all? I should have more time over the weekend to dig into this. But feel free to take over.

🍺

@joonhocho
Copy link
Author

joonhocho commented Jun 15, 2016

I need some time to really learn how routers work. I took some time last night, but there seem to be lots of things to learn. I will spend next few days digging sources, and will update this thread as I make any progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants