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

react-router@4 compatibility #193

Closed
ooflorent opened this issue Sep 7, 2016 · 20 comments
Closed

react-router@4 compatibility #193

ooflorent opened this issue Sep 7, 2016 · 20 comments

Comments

@ooflorent
Copy link
Contributor

ooflorent commented Sep 7, 2016

The current API of react-router-relay seems to be incompatible with react-router@4.
Is there any plan for supporting it?

@taion
Copy link
Member

taion commented Sep 7, 2016

No plans. React Router v4 takes an approach to routing that's fundamentally incompatible with what I want to accomplish here w/r/t data fetching.

@taion taion closed this as completed Sep 7, 2016
@ooflorent
Copy link
Contributor Author

Thanks for the clarification. So, the goal is to stick with RR3? Drop RR in favor of Relay router?

@taion
Copy link
Member

taion commented Sep 8, 2016

There will be something different coming.

@jameslk
Copy link

jameslk commented Sep 17, 2016

@taion I would love to know what that is. I depend on your library, as well as the isomorphic relay router library. Hopefully it won't involve a terrible amount of churn. It's a pain having to deal with that in React Router as it is. Are you planning on making an announcement in the short term?

@taion
Copy link
Member

taion commented Sep 17, 2016

@jameslk

For the time being, please continue to use RRv2. That's what I'll be doing for the time being, as RRv2 largely satisfies my needs, and it's not like it's going to stop working just because there's a release out with a larger version number.

If you want to track what's happening, keep an eye on https://github.com/4Catalyzer/found/tree/initial-impl and https://github.com/4Catalyzer/farce. The latter looks roughly how I want it to look, I think, but the former is under active development – I haven't even figured out how all the API points should tie together yet.

found is going to look pretty similar to RRv2, but the insides will be a bit different and should make it easier to show loading indicators when using async getComponent. It won't support getChildRoutes though.

@sibelius
Copy link

@ooflorent @jameslk u can use this snippet: https://gist.github.com/janicduplessis/f513032eb37cdde5d050d9ce8cf0b92a

that will wrap any route component inside a Relay.Renderer, so u can easily use with React Router 4

@taion
Copy link
Member

taion commented Oct 20, 2016

That's a quite bad idea, BTW – it will make all of your loads unavoidable waterfall.

Unfortunately the sort of component composition in RRv4 just isn't a good model for async data fetching with routing.

So don't do that unless you actively want to delivery poor UX.

@bsr203
Copy link

bsr203 commented Oct 20, 2016

I recently migrated from react-router to http://grahammendick.github.io/navigation/
There are plenty of samples, including relay, hot reloading.

The migration was smooth, and the library is super tiny.

I also like to hear @taion 's view on the library as he is one of the most knowledgeable in react + relay + react-router. thanks in advance.

@AnderssonChristian
Copy link

@taion Any updates on the RRv4 support for Relay applications? Or is RRv3 still the recommended approach?

@taion
Copy link
Member

taion commented Mar 13, 2017

Nothing from earlier in the thread has changed.

@brysgo
Copy link

brysgo commented Sep 20, 2017

@taion - I know this is a really old comment that I made here:

relayjs/relay-examples#31

But I'm resurfacing it because I want validation that I'm not doing something that is going to get me into trouble later.

Ah, I see. Yeah, my routes just end up looking like this:

<Route
  path={"/nested/:thingId/thing"}
  component={props => <Thing data={data} {...props} />}
/>

And that is how I orchestrate my fragment containers.

I have been using the above technique to use relay-modern with react-router@4 without nesting QueryRenerer's. Any feedback on the approach would be much appreciated. (For some extra context, the 'data' variable is relay fragment data.)

@taion
Copy link
Member

taion commented Sep 20, 2017

The issue is more like – suppose you have multiple routes that need different data.

Presumably you're doing the data fetching in a parent component, but how does that parent component know what query it should run, and which data it should fetch?

Like it's fine if you only have a single possible route, but otherwise you need to determine all your data dependencies in the ultimate parent, and if there are multiple possible sets of child routes with different data dependencies, you somehow need to sort all of that out on the parent.

@brysgo
Copy link

brysgo commented Sep 20, 2017

I see, could that be solved with refetch containers somehow?

@brysgo
Copy link

brysgo commented Sep 20, 2017

Each component, just needs to know which of it's immediate children will be rendered. Then it would have to refetch with different fragments spread. I can see how it gets complicated.

@taion
Copy link
Member

taion commented Sep 20, 2017

Plus you're refetching, and these are exactly the waterfalled requests that by presumption we don't want.

@brysgo
Copy link

brysgo commented Sep 20, 2017

So is found-relay your recommended solution?

@taion
Copy link
Member

taion commented Sep 20, 2017

Correct.

It's roughly the same API as React Router Relay, but has better support around things like async components, showing loading indicators, and redirecting or hitting 404s based on Relay data.

@haleykoike
Copy link

@taion Thoughts on react-router-config? It gives RRv4 the ability to have static route configuration again.
https://github.com/ReactTraining/react-router/tree/master/packages/react-router-config

With react-router-config, would there be some consideration in making react-router-relay compatible with RRv4?

@taion
Copy link
Member

taion commented Sep 27, 2017

It's okay-ish for some use cases, but ultimately it's not a great solution. It's feature-incomplete as a config-based router.

In essence you end up just dropping all the core routing functionality in React Router v4, and rebuilding in a sort of messy way the v3 API. You don't get any of the benefits of the dynamic component-based routing, but you also don't get the sort of full-fledged config-based routing API, because it's sort of awkward to build on top of that system.

In the end, all you really get is a weird sort of wrapper around path-to-regexp.

My recommendation remains to switch to Found and Found Relay.

@AlvinLaiPro
Copy link

Marked

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

No branches or pull requests

9 participants