Is it recommended to fetch in effect or should it be imperative

[otakustay]

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:

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:

{
    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.

[Bear-Foot]

So, the approach I'm going to provide relies on my personnal experience, but tries to take every concern you have into account. More than happy to discuss any part of it.

I understand the appeal of using the useEffect diffing system, and I will go for that. However, I would never EVER have it inside my poor List. In my opinion, it should remain pure, taking only the results (maybe some other displaying configuration props, but not relevant here) as a prop. So, I would not consider your 2 first examples really separating view and logic.

Forked your 2nd example, you can follow along here https://codesandbox.io/s/826zymymkl I move the useEffect to the ListContainer. I'm annoyed by eslint that I don't provide onLoadResult to the deps, because I know its constant, but nevermind. And my List is pure, preventing any renders of ListContainer regarding the change or parameters to affect the render of List.

Because you seem concerned with extra renders, in your switchFilter action, I "batch" the change of the filter with the resetting of the page to 1.

That is pretty much all I would change. You will need to make ListAutoFetcher more generic to apply it to others lists and stuff, but again, it's another topic.

[hanzhixing]

In my opinion, the most important question is “As a state machine, the js environment changes because of signals from outside(from location, network io, screen interaction, etc...), or because of its own lifecycle(like renders)?”. When the effect is called, even the dom did not change.

When paging, we need extra filter state, because the api is designed so. When submitting filter form, we need extra page state，also because of the api spec. These are logic about data(business logic), not about the view.

So I vote 3rd solution, even if it is without our new super hooks.

By the way, the name ‘hook’ is so evil.

For a long time, names like this are always for user context, not for engine it self.

Why they are not named as beforeRender, afterRender...?

[gaearon]

The longer-term idiomatic solution to this problem is Suspense, which would not rely on any of those things. I know it's not very satisfying since it's not something you can use today, but I just want to make it clear why it's somewhat awkward with all of these approaches. Neither of them natively matches the React paradigm very well.