ryanflorence/react-router-mega-demo

Discussion: fetchData API

geelen opened this issue · 8 comments

Hi Ryan, let me start by saying I love love love this repo. I have made little starts at doing something similar but never gotten very far, and this looks fantastic. I particularly <3 the webpack config and your approach to async data on the server side. But on the latter I had a couple of questions:

The way you've got it, each top-level Handler needs to declare a fetchData if it has async dependencies, is that correct? If that Handler was, say, a dashboard, that had to pull in data from multiple places, that all needs to be done in the Handler then passed down to each component? There's no way for the components to 'bubble up' their dependencies on async data so it doesn't all need to be done at the one level?

@geelen, by my understanding, you should return a Promise to fetchData. When there is more to solve than one request, you should do more requests and so you can solve that Promise.

Take Contact handler for example: It returns a call to api that returns a promise.

fetchData: function(token, params, query) {
      return new Promise(function(resolve) {
            api.get( /* … */)
            .then((result)=> {
                  /* … */
                  return api.get(/* … */);
             })
            .then(resolve) ;
      });
}

Yes, but then there can only be one "level of fetchData", so the topmost component would have to know what all its child components need, instead of each component declaring its own fetchData.

I do agree!

So, where should be the source of all our data?
How do we should arrange our data?
Some say that we could have a AppState that describes everything, and any dependency.
I don't know, for me it does look very coupled. Although, it doesn't feel very well either every component asking data, worst when running on server, because the rendering should occur only at the end of all fetch, if it isn't centralized, there should be at least a queue for requests from a single user.

Anyway, I'm just rambling here, but i think there is no single answer, it varies accordingly to the case and team.

You could pass a fake <Handler/> to your fetchData and recurse all the children to find any fetchData statics on those components then map them all to a hash of key:promises, and then actually render it with the data:

var all = require('when/keys').all;

Router.run(routes, location, (Handler, state) => {
  var traversableComponent = <Handler/>;
  fetchData(traversableComponent, state).then((data) => {
    React.render(<Handler data={data}/>, document.body);
  });
});

var fetchData = (component, routerState) => {
  var { params, query } = routerState;
  var promises = {};
  recurseRenderTree(component, (child) => {
    if (child.type.fetchData)
      var name = child.type.dataKey;
      promises[name] = child.type.fetchData(routerState);
    return promises;
  }, promises);
  return promises;
};

var recurseRenderTree = (parent, fn) => {
  fn(parent);
  if (parent.props.children)
    React.Children.forEach(parent.props.children, (child) => {
      if (child.props.children)
        recurseRenderTree(child, fn);
      else
        fn(child);
    });
};

And now every component in the system simply needs to do this:

var Thing = React.createClass({
  statics: {
    fetchData () {
      return somePromise();
    },

    dataKey: 'stuff'
  },

  render () {
    var list = this.props.data.stuff.map((thing) => {
      return <li>{thing.name}</li>;
    });
    return (
      <ul>{list}</ul>;
    );
  }
});

While this is pretty interesting, I think there's value in just your handlers defining the initial render's data dependencies. Sprinkling it all over the app hierarchy is reminiscent of the inline SQL queries I did when I first started programming, makes it hard to identify them.

Route handlers are "entry points" into your app. I like having them define the data dependencies so all I have to do is look at my entry points to know my data, instead of grepping my app for fetchData.

While this is pretty interesting, I think there's value in just your handlers defining the initial render's data dependencies. Sprinkling it all over the app hierarchy is reminiscent of the inline SQL queries I did when I first started programming, makes it hard to identify them.

An interesting case here is when you want to use something like lazy chunk loading from Webpack (or similar).

In this case you might need to have a shim component which acts as a loader while the chunk is downloading—initially rendering an indeterminate spinner or something—and is then replaced with the loaded component from the downloaded chunk.

In this case your shim component will be as far as the router can walk when initially run, unless you fake it and make the shim smart enough to not resolve its fetchData until the wrapped async component’s fetchData also resolves (or something).

nmn commented

@rpflorence In general I don't mind the central approach to data with flux. And for many apps that works just fine.

But composing components is one of the things that makes React great. No one would argue that a google maps component should need a to fed any data other than simple props.

Similarly, there are often components that are independent from the rest of the app, and should really fetch their own data in an ideal world. However they also need to rendered on the server-side. This makes things complicated. There has got to be a better solution.

Relay and GraphQL seem to be the missing links.

With the proposed solution by @ryanflorence there is still the issue of query/data duplication. Sub-components A and B both might need resource C. We'd fetch resource C twice, as separate queries. For larger hierarchies this defeats the purpose of defining async dependencies at a sub-component level.. Unless we cache the api calls (which the current demo does), however I'm not sure if this is an acceptable workaround. For if the calls are made in quick succession (such that the original call hasn't been returned yet) it will result a cache miss for the subsequent calls. This gets even worse for nested data.

Some of these issues could be remedied by introducing a generic, singleton Store. fetchData would return Actions.getResource("User.projects"). The Store could then return an existing promise (if one exists) or create a new one. But then we're still left with a potentially inefficient query (or queries) to fetch the resource in the given shape.

A solution to this seems rather complex and perhaps – but hopefully not – out of the scope for this project. Let me know if you think I'm wrong. I'd love to find a solution to this.