-
-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Description
Hi,
I'm currently working on a big Webapp (Typescript / React / Redux) where we have some data to fetch. Do do so, we have tried different things:
Let's take the following case : the state is
{ user: /* a user */, account: null }
and we have the component that display the list of account for the current user.
First attempt: componentDidMount
So at first, we did something like this :
// In the component that need accounts
componentDidMount() {
if (this.props.accounts === null) {
dispatch(fetchAccounts(this.props.user))
}
}
This solution had many problems:
- If more than one component need accounts and are not
parent <> child
then thefetchAccount()
is dispatch twice. - Even with only one component, while the data is fetching we re-fetch at each render.
- Since this component need account to render more than
no data
orloading
we do a render just to trigger the fetch action. - We need to pass the user to the component while it's only needed for the request
Second attempt: Improve the state
Let's solve the first two problem by adding more info in the state.
State is now:
{
user: /* a user */,
account: {
status: 'void',
content: null,
fetchedUser: null
}
}
// In the component that need accounts
componentDidMount() {
if (this.props.accounts.status === 'void' || this.props.accounts.fetchedUser !== this.props.user) {
// This set `accounts.status` to `loading` and set user in `accounts.fetchedUser`
dispatch(fetchAccounts(this.props.user));
}
}
Now we have prevent useless fetching but the code above need to be copied if to component that need accounts are not parent <> child
.
To solve this, let's make fetchAccounts
a thunk and put the logic inside:
// In the component that need accounts
componentDidMount() {
dispatch(fetchAccountsIfNeeded(this.props.accounts, this.props.user));
}
It's better, but we still have some copy/past
to do...
Wait, why is this in a component ?
In fact the real problem was that the fetch request has nothing to do in a component. Because fetching account data depend on 2 things : the user in state and the need to render a component that need accounts.
And since the render is predictable from the state, the only thing we need to fetch accounts is the state.
Third Attempt: a reducer after the reducer
What we needed was something that take the result of the reducer and do all the logic / resolve all state dependencies.
So we create a createStoreEnhancer
that take a postReducer
as argument, then the enhancer would return a reducer that work that way :
- call original reducer
- call the postReducer
- is the postReducer has changed the state call the postReducer again.
The result was that while the postReducer find something to change, we call the postReducer. This way we get some sort of propagation.
But this approach has 2 major issues:
- We did side effect in post reducer (that was our initial question: where should we put side effect) which was really bad because it mean we were doing side effects in the reducer.
- The state was mutated in only one cycle, so you dispatched an action and then you get a lot of changes in the state without knowing where it came from...
Fourth Attempt: Between the reducer
and listener()
Since inside the reducer was not a good idea (at all), I needed to put my "postReducer" on step after so I changed my createStoreEnhancer
instead of retuning a new reducer, I will now return a
new subscribe
:
function subscribe (listener) {
return initialSubscribe(() => {
function dequeuePostReducerDispatchQueue() {
store.dispatch(postReducerDispatchQueue.shift());
}
function dispatchFromPostReducer(action) {
postReducerDispatchQueue.push(action);
}
if (postReducerDispatchQueue.length > 0) {
dequeuePostReducerDispatchQueue();
return;
}
// Run postReducer
postReducer(dispatchFromPostReducer, state);
if (postReducerDispatchQueue.length > 0) {
dequeuePostReducerDispatchQueue();
return;
}
listener();
});
}
The result is quite the same than in third attempt but this time action are dispatched and side effects are not in the reducer.
BTW, this is the current solution.
So is this the right implementation ? (spoiler: nope)
No, this is still not right. in fact it is almost as bad as the previous one.
This solution currently works on our webapp because we use react-redux, so only one call is made to subscribe
and the postReducer is apply only once.
But if we add just one listener to the store... Then post reducer are apply twice and everything breaks 💥
future fifth attempt: The correct one
I was first to early (in reducer) then too late (in subscribe) so the correct answer was between these two. In fact to work as expected I should directly subscribe to the store in createStoreEnhancer
and then create my own subscribe method.