Repository: react
Issue: Is it recommended to fetch in effect or should it be imperative
Labels: Type: Discussion,Component: Hooks

In out team we encountered a explosive discussion on how we should handle the relationship of a fetch and its parameters, after searching in community I still find various solutions to this, I'd like to raise this discussion to find a best practive.

## Background

Suppose we have a simple list view like:

<img width="687" alt="Jietu20190402-130206@2x" src="https://user-images.githubusercontent.com/639549/55377435-87268b80-5547-11e9-97a8-313a5713ced3.png">

Whenever user types keyword in textbox and clicks "Search" button, or they change the page number, we should fetch a new list from remote and render it in table.

We use redux to manage global state of this simple app, the store is structured as:

```js
{
    filter: '',
    pageIndex: 0,
    results: []
}
```

We developed a total of 3 solutions to demonstrate how the change of `filter` and `pageIndex` should cause a fetch of `results`.

## Use effect and separation of view and logic

This is the first demo: https://codesandbox.io/s/20x1m39w00

In this implementation we tried to:

1. Utilize `useEffect` to trigger a fetch when any parameter changes.
2. Do not pass any parameter as prop to `components/List` component.

In my point of view, I like this solution best because:

1. It have a very clear separation of view and logic, `components/List` does not receive any redundant props such as `filter` or `pageIndex`.
2. It theoretically treat a callback prop as a normal one, make it a dependency of `useEffect`.
3. It works in a **reactive** way, which means "we trigger a fetch not because the action taken from user, only because the change of state".

Still we have concerns about it:

1. It obviously triggers more render and updates because change of `filter` or `pageIndex` does not dispatch `FETCH_RESULTS` immediately, this cause a sync dispatch in effect which we previously avoided by `no-set-state-did-update` rule.
2. We create a state update from another state update, this "chaining" is not clear enough for developers and may cause unwanted infinite loop.

## Use effect and params together

The second demo is much like the first one: https://codesandbox.io/s/54o1rjvyv4

The only change is we pass `filter` and `pageIndex` to `components/List`, in this case we believe **effect is a part of component** so that every dependencies used to form an effect should be passed as prop.

This solution gives a more clear view of what is used to fetch data in `components/List`, this is 
 a highly adopted solution in community, however we're not sure this is recommended officially.

## Imperative action to fetch data

As opposed to previous, this is our third demo: https://codesandbox.io/s/p5yv48x97x

In this solution we changed our thought and implement the app in a more "redux way":

1. We trigger the fetch on user interactions, either click on "Search" button or change the page number, however either interaction only provides its own parameter, we don't provide `pageIndex` when "Search" button is clicked.
2. We have a thunk which computes a new parameter object based on current state using `getState()` function, a `FETCH_RESULTS` action is dispatched.
3. We have several reducers to observe `FETCH_RESULTS` action and updates corresponding parameter in global state.
4. Fetched list is connected to `components/List` component, this component now is a pure presentational component, no lifecycle effect is involved.
5. To solve the first fetch when application is mounted, we create an `containers/App` container component.

By doing these we eliminated the "chaining state update" issue, however it introduces several concerns:

1. If we add more user interactions in the future, the `loadResults` thunk could be more and more complex.
2. The use of `getState` in `redux-thunk` is not highly recommended in community, we found some articles stating that developers should avoid to use it in most cases.
3. We can't explain the exist of the `containers/App` container only to trigger a fetch on mount, thee `useEffect` take no dependencies and `exhaustive-deps` rules complains about it, not paring mount and update is also a big uncomfortable point to us.
4. Trigger fetch from user interactions is what we called "imperative", we're confused about whether a reactive framework like react recommends imperative programming.

------

Since we are not able to get a conclusion for a very long time, we decide to raise this issue for more discussion to find a better solution to these very common use cases.