Need to handle multiple services

[ZeroCho]

Note: This is just a template, so feel free to use/remove the unnecessary things

Description

- Type: Enhancement

Enhancement

Reason to enhance/problem with existing solution
Currently, preRenderMiddleware only can handle one data. It should be possible to handle multiple data as an app scales.

Suggested enhancement
Need to change some structure of preRenderMiddleware.js or fetch-data scripts to accept array of services.

[choonkending]

@ZeroCho Good concern!

I think that if you have control over the API, each route on load should only fetch once. Perhaps the responsibility of fetching from different sources, should lie on the API side of things (i.e. the server).

However, if that is not possible, I think we can then modify the fetchData method to fetch data from multiple endpoints and perhaps promise.all them?

Is that an acceptable solution for you?

[ZeroCho]

I'm now running my blog based on this package, and I changed my blog's structure after your update. What is your opinion for my codes? @choonkending

Everything is similar except for following two files, and fetch-data/fetchPost.js file shows what I mean by multiple data. It loads both post and comments. I directly use actions in fetchPost.js

fetch-data/fetchPost.js

import { loadPost, loadComments } from '../reducers/posting';

export default [
  (params) => loadPost(params.id),
  (params) => loadComments(params.id),
];

preRenderMiddleware.js

export default function preRenderMiddleware(dispatch, { routes, params }) {
  const matchedRoute = routes.map(route => route.fetchData);
  const funcs = [];
  matchedRoute.forEach(dataArray => {
    dataArray.forEach(data => {
      funcs.push(data);
    });
  });
  const promises = funcs.map(need => dispatch(need(params)));
  return Promise.all(promises);
}

[choonkending]

Excellent explanation + code snippet. Thanks for that (and also getting on board the changes so quickly)!

Personally I think it's a matter of preference at this point - although I would lean towards doing the following:

const fetchData = params => {
   const p1 = loadPost(params.id);
   const p2 = loadComments(params.id);
   return Promise.all([p1, p2]);
};

So you will be able to use preRenderMiddleware as is - it makes it a bit simpler, and we won't need to rely on dispatch.

Does the above help with your use case?

I'd love to see your blog in our list of example apps!

[StefanWerW]

Question: in @choonkending exemple, how can I properly reduce the data fetched, if both will be dispatched as { type: types.REQUEST_SUCCESS, data } ? How could one sort Post from Comments on this example?

[DraconPern]

This is the solution I came up with. The idea is to pass in the store to the fetch function and use redux-thunk to dispatch a route specific action.

So, first pass store to preRenderMiddleware so the fetch function can have access.

app/middlewares/preRenderMiddleware.js

function preRenderMiddleware({ routes, location, params }, store) {
  const matchedRoute = routes[routes.length - 1];
  const fetchDataHandler = matchedRoute.fetchData || defaultFetchData;  
  return fetchDataHandler(params, store);
}

app/client.jsx

preRenderMiddleware(this.state, store)

app/server.jsx

preRenderMiddleware(this.state, store)

Then, in the fetch function, also dispatch GET_TOPICS_SUCCESS. Note that I import types

app/fetch-data/fetchVoteDatajs

import * as types from 'types';

const fetchData = (params, store) => {
  return voteService.getTopics()
    .then(res => {
      store.dispatch({type: types.GET_TOPICS_SUCCESS, data: res.data});
      return res.data;
    });
};

Finally, change the reducer so that instead of handling the general REQUEST_SUCCESS, handle GET_TOPICS_SUCCESS.

app/reducers/topic.js

const topics = (
  state = [],
  action
) => {
  console.log('topics action.data');
  console.log(action.type);
  console.log(action.data);
  switch (action.type) {
    case types.GET_TOPICS_SUCCESS:
      if (action.data) return action.data;
      return state;

With this solution, if you add additional data fetching, then the state will actually be correct.

[DraconPern]

I also want to figure out how to show that the fetch fails. Let's say network error or the service goes down. Right now, the UI just does nothing.

I think generically..
app/client.js in onUpdate add a catch

preRenderMiddleware(this.state, store)
  .then(data => {
    return store.dispatch({ type: types.REQUEST_SUCCESS, data });
  })
  .catch(error => {
    return store.dispatch({ type: types.REQUEST_FAILURE, data: error});
  })

[GGAlanSmithee]

I really like the implementation that @ZeroCho is suggesting. I am using one much like it in an app which is unrelated to (not built on) this project.

The main thing is that it removes data-fetching coupling for nested routes:

<Route path='/root' fetchData={fetchRootData}>
    <Route path='/nested' fetchData={fetchNestedData}/>
</Rotue>

Navigating to root/nested would preload both fetchRootData and fetchNestedData without them knowing anything about each others fetch methods.

[DraconPern]

I have the same concern as @StefanWerW . If you build another data type, what happens is you'll have two reducer handling REQUEST_SUCCESS. I used redux dev tools and I noticed that if I load some other data, topics gets overwritten because it is also handling REQUEST_SUCCESS. Now imagine the case where I load a component outside of navigation, say a hover over. It's going to now have bad data.

[GGAlanSmithee]

@DraconPern yeah it would have to be changed to use different actions, like TOPICS_REQUEST_SUCCESS etc.

[slavab89]

I have a question regarding this whole flow.
I understand what the purpose of preRenderMiddleware but for some odd reason i'm not able to follow what's happening on the server during initial load.

One of the pages in my app requires 2 things from the API server, lets say classes and students. The thing is that they depend on each other, i want to get the students of a specific class that i'm loading.
I wanted to hold off the loading of the initial page until i have both but i'm not sure how to do that since students and classes are different recuders therefore require different actions. But how do i dispatch actions that will actually 'wait' and be part of the preRenderMiddleware?
I think i'm missing something =[

[slavab89]

Just wanted to update on what i ended up doing on my end :) Would be great if you tell me what you think (its mostly based on what @DraconPern did)
So i've taken the advice to pass the dispatch function into preRenderMiddleware + to the fetchData file that handles the initial load.
There, instead of using axios or request or whatever module, i've just required actions that i want to happen and dispatched them.

const fetchData = (params, dispatch) => {
  return dispatch(getClasses()).then((res) => {
    const promises = [];

    promises.push(dispatch(getStudents()));
    promises.push(dispatch(getTeachers()));
    return Promise.all(promises);
  });
};

Actions can be promises or regular function, doesnt really matter as promiseMiddleware takes care of their logic.
Moreover, i no longer need the 'service' (the one that does the network request) because the action takes care of what's needed.
The REQUEST_SUCCESS REQUEST_FAILURE actions are used just as an indication of initial load

[GGAlanSmithee]

Here is my take on it. (pretty much a combo of @slavab89 and @ZeroCho)

import {flatten} from 'lodash'

export default(dispatch, {routes, params}) => {
    const dataFetchers = flatten(routes.filter(({fetchData}) => !!fetchData))

    return Promise.all(dataFetchers.map(({fetchData}) => dispatch(fetchData(params))))
}

where dispatch is from the store and a fetchData is either falsy (null, undefined etc), an action (function, Promise) or an array of either:

<Route>
    <Route path='/' component={App} fetchData={fetchSomethingAction}>
        <Route path='/users' component={Users} fetchData={fetchUsersAction}/>
        <Route path='/about' component={About}/>
    </Route>
</Route>

For some added confusion here is a one-liner version 💃

export default (d, {routes:r, params:p}) => Promise.all(flatten(r.filter(({fetchData:f}) => !!f)).map(({fetchData:f}) => d(f(p))))

[slavab89]

@GGAlanSmithee Seems cool. I like that
2 things that i wanted to ask:

1. Why do you need the flatten? You're running on the routes and get back an array with 1 level only, no?
2. I dont have it on my app so didnt do it but what if i have a nested route? I'll need to check the ChildRoutes as well, no? (Again, i dont have that so i didnt check how it works as React may lay out all the route components in the 'routes' object already heh)

[ZeroCho]

@slavab89 I think flatten is for nested route, although i haven't test it yet. If it isn't, you need to apply another way to check nested route.

[GGAlanSmithee]

@slavab89 @ZeroCho In my example, flatten will flatten fetchData if it's an array, which allows for

<Route path='/mypath' component={MyComp} fetchData={[a, b, c, d]}/>

Regarding nested routes, I was convinced that the routes param was already flat, but if not, you could use flatten for that as well.

EDIT: Actually looking at it, it is indeed flattening the routes, well spotted @ZeroCho. This is an error though, as I intended to flatten fetchData. Not at home now, but will update it later today.