relay-tools/react-router-relay

GraphQL error gets stuck in parent route

coolov opened this issue · 11 comments

I have a set of nested routes, like this:

  <Router
    history={browserHistory}
    // hack to get this example working
    // see https://github.com/relay-tools/react-router-relay/issues/192
    render={applyRouterMiddleware(useRelay.default || useRelay)}
    environment={environment}
  >
    <Route
      path="/"
      component={Shell}
      queries={{user: () => Relay.QL`query { user }`}}
      render={({ error, props, element }) => {
        if (error) {
          return <div>Error!</div>;
        }
        if (props) {
          return React.cloneElement(element, props);
        }
        return <div>Loading</div>

      }}
    >
      <Route
        path="/home"
        component={Home}
        queries={{ home: () => Relay.QL`query { home }` }}
      />
    </Route>
  </Router>

If GraphQL returns an error in the home query, I still want to render the user query that is in the parent route of the home query.

Currently, the error in the home query is returned in the render method of the parent route (the one containing the user query), and there is no way to access the props from the user query.

How do I render the result of the user query when the home query fails?

Example to reproduce this behavior:
https://github.com/coolov/react-router-relay-nested-error

taion commented

react-router-relay fetches all the data as a single query, so it can't do per-route error state without a rewrite. Found Relay actually supports this, if that's relevant.

nodkz commented

@coolov for your task react-router-relay is not applicable, you should use Relay.Renderer directly, eg.: https://gist.github.com/nodkz/ba89637af2485ea281eeb89b3ed25160

@taion --- with nested routes, each query is fetched separately. in my example i see one request for the user query and one for the home query:

screen shot 2016-12-13 at 11 32 45 am

screen shot 2016-12-13 at 11 32 55 am

@nodkz in your example, how do you handle the URL routing?

nodkz commented

@coolov as usual with any routing which you use (react-router v2, v3, v4) like in their docs.

My example just traverse a routes tree and wraps your components by Relay.Renderer if for your route provided additional queries property (which contains Relay.Route). So when route reaches such path it just runs render of Relay.Renderer (and it internally fetch data from server/cache).

addLoaderForRelayComponents method very simplified but working substitution for react-router-relay (without aggregation of queries and variables and as result of errors from server). So each query runs independently and also allows to you make queries with variables on top level, eg:

// in my case I use `node` for obtaining `CompanyPage` firstly from cache if possible
export class CompanyPageRelayRoute extends Relay.Route {
  static routeName = 'CompanyPage';
  static prepareParams = params => ({
    id: toGlobalId('Company', params.companyId),
  });
  static queries = {
    company: () => Relay.QL`query { node(id: $id) }`,
  };
}


// somewhere in your routing
{
   path: ':companyId',
   component: CompanyPage,
   relayRoute: CompanyPageRelayRoute,
}

thanks @nodkz, this is very helpful!

taion commented

That's not quite right – you don't want to separately wrap components in Relay.Renderers when using nested routes, because that will force a waterfall in data requests. See https://medium.com/@cpojer/relay-and-routing-36b5439bad9, namely

This means that on every step of the route tree, Relay has to fetch all the data for that level until it can proceed to fetch data for the next level in the route tree. Relay is trying to solve the multiple-API-request problem by using GraphQL but if you are using the above solution you will not benefit from Relay’s optimizations.

Separately, if you are not using a batching network layer, then you will see an HTTP request per GraphQL query, but they will be issued in parallel anyway.

nodkz commented

Moreover, a batching network layer (for my example above) should have specific logic to catch requests some period of time (eg. via setTimeout) and on the next tick send combined request.

@coolov current batch implementation in https://github.com/nodkz/react-relay-network-layer does not solve this (for every nested route will send separate request). But perfectly works for react-router-relay.

So all solutions have pros and cons. Anyway, after trying all current solutions, you may write your own.

taion commented

It doesn't need to batch over time; the separate requests all show up in the same array of requests that the network layer receives.

nodkz commented

For my example above needs to batch over time. Just checked with my internal app. Cause for every react route it uses different Relay.Route and Relay sends them in different calls of sendQueries(queryRequests).

But I have a specific view in my app (like gmail) so loading in different requests fit my needs.

taion commented

If you're nesting <Relay.Renderer> components, that's not enough – the child <Relay.Renderer> won't start fetching its data until after the parent <Relay.Renderer> finishes, in the case where you show some loading indicator or otherwise don't render the children until the data are ready. This is because that child <Relay.Renderer> can't fetch its data before its parent renders it.

That is what react-router-relay addresses, though the implementation in Found Relay is quite a lot better, and I'd consider a PR to backport it here, to the extent that it's workable with the React Router API (I'm not sure it is).