I'm building an application with React and Redux.
I have an Account component that fetches data from the server in the componentWillMount
method. While the data is being fetched, the component must show the "loading" text, so I've added the "isFetching" property to the account reducer. This property is set to true while data is fetching from the server.
The problem is, that while data is being fetched, the value of the "isFetching" property in the render
method is false, while at the same time the value of store.getState().account.isFetching
is true (as it must be). This causes the exception, because this.props.isFetching
is false, so the code is trying to show the this.props.data.protectedString
while the data
is still being loaded from the server (so it is null).
I assume that the mapStateToProps bind some wrong value (maybe the initial state), but I cannot figure out why and how can I fix it.
Here is my AccountView code:
import React from 'react';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import * as actionCreators from '../../actions/account';
class AccountView extends React.Component {
componentWillMount() {
const token = this.props.token;
// fetching the data using credentials
this.props.actions.accountFetchData(token);
}
render() {
console.log("store state", window.store.getState().account); // isFetching == true
console.log("componentState", window.componentState); // isFetching == false
return (
<div>
{this.props.isFetching === true ? <h3>LOADING</h3> : <div>{this.props.data.protectedString}</div> }
</div>
);
}
}
const mapStateToProps = (state) => {
window.componentState = state.account;
return {
data: state.account.data,
isFetching: state.account.isFetching
};
};
const mapDispatchToProps = (dispatch) => {
return {
actions: bindActionCreators(actionCreators, dispatch)
};
};
export default connect(mapStateToProps, mapDispatchToProps)(AccountView);
Account reducer:
const initialState = {
data: null,
isFetching: false
};
export default function(state = initialState, action) {
switch (action.type) {
case ACCOUNT_FETCH_DATA_REQUEST:
return Object.assign({}, state, {
isFetching: true
});
case ACCOUNT_RECEIVE_DATA:
return Object.assign({}, state, {
data: action.payload.data,
isFetching: false
});
default:
return state;
}
}
Actions:
export function accountReceiveData(data) {
return {
type: ACCOUNT_RECEIVE_DATA,
payload: {
data
}
};
}
export function accountFetchDataRequest() {
return {
type: ACCOUNT_FETCH_DATA_REQUEST
};
}
export function accountFetchData(token) {
return (dispatch, state) => {
dispatch(accountFetchDataRequest());
axios({
// axios parameters to fetch data from the server
})
.then(checkHttpStatus)
.then((response) => {
dispatch(accountReceiveData(response.data));
})
.catch((error) => {
//error handling
});
};
}
This is how I'm creating the store:
import { applyMiddleware, compose, createStore } from 'redux';
import { routerMiddleware } from 'react-router-redux';
import rootReducer from '../reducers';
export default function configureStore(initialState, history) {
// Add so dispatched route actions to the history
const reduxRouterMiddleware = routerMiddleware(history);
const middleware = applyMiddleware(thunk, reduxRouterMiddleware);
const createStoreWithMiddleware = compose(
middleware
);
return createStoreWithMiddleware(createStore)(rootReducer, initialState);
}
And in index.js:
import { createBrowserHistory } from 'history';
import { syncHistoryWithStore } from 'react-router-redux';
import configureStore from './store/configureStore';
const initialState = {};
const newBrowserHistory = createBrowserHistory();
const store = configureStore(initialState, newBrowserHistory);
const history = syncHistoryWithStore(newBrowserHistory, store);
// for test purposes
window.store = store;
My code is based on this example - https://github.com/joshgeller/react-redux-jwt-auth-example
The code looks the same, but I've changed some places because of new versions of some modules.
Isn't your ternary statement switched? Your render function has this:
{this.props.isFetching === true ? <h3>LOADING</h3> : <div>{this.props.data.protectedString}</div> }
and your initialState in your reducer is this:
const initialState = {
data: null,
isFetching: false
};
That would default to this.props.data.protectedString
immediately on mount.
You should always ask yourself these two questions when you are fetching data with react & redux:
- Are my data still valid ?
- Am I currently fetching data ?
You have already answered the second question by using the isFetching
but the first question remains and that is what causing your problem.
What you should do is to use the didInvalidate
in your reducer (https://github.com/reactjs/redux/blob/master/docs/advanced/AsyncActions.md)
With didInvalidate
you can easily check if your data are valid and invalidate them if needed by dispatching an action like INVALIDATE_ACCOUNT
.
As you haven't fetched your data yet, your data are invalid by default.
(Bonus) Some examples of when you might invalidate your data:
- The last fetch date is > X minutes
- You have modified some data and need to fetch this data again
- Someone else has modified this data, you receive the invalidation action through Websockets
Here is how your render should look like:
class AccountView extends React.Component {
componentDidMount() { // Better to call data from componentDidMount than componentWillMount: https://daveceddia.com/where-fetch-data-componentwillmount-vs-componentdidmount/
const token = this.props.token;
// fetching the data using credentials
if (this.props.didInvalidate && !this.props.isFetching) {
this.props.actions.accountFetchData(token);
}
}
render() {
const {
isFetching,
didInvalidate,
data,
} = this.props;
if (isFetching || (didInvalidate && !isFetching)) {
return <Loading />; // You usually have your loading spinner or so in a component
}
return (
<div>
{data.protectedString}
</div>
);
}
}
Here is your Account reducer with didInvalidate
:
const initialState = {
isFetching: false,
didInvalidate: true,
data: null,
};
export default function(state = initialState, action) {
switch (action.type) {
case INVALIDATE_ACCOUNT:
return { ...state, didInvalidate: true };
case ACCOUNT_FETCH_DATA_REQUEST:
return {
...state,
isFetching: true,
};
case ACCOUNT_RECEIVE_DATA:
return {
...state,
data: action.payload.data,
isFetching: false,
didInvalidate: false,
};
default:
return state;
}
}
Below your new lifecycle:
1. First render
- Description: No fetch happened yet
- Reducers:
{ isFetching: false, didInvalidate: true, data: null }
- Render:
<Loading />
2. componentDidMount
- Description: The data is invalidated && no fetching --> go fetch data
3. Function called: accountFetchData (1)
- Decription: Notify reducers that you are currently fetching and then fetch the data asynchronously
- Dispatch:
{ type: ACCOUNT_FETCH_DATA_REQUEST }
4. Account Reducer
- Description: Reducers are notified of the dispatch and modifies their values accordingly
- Reducers:
{ isFetching: true, didInvalidate: false, data: null }
5. Second render
- Description: Goes a second in the render because the Account reducer changed
- Reducers:
{ isFetching: true, didInvalidate: false, data: null }
- Render:
<Loading />
6. Function called: accountFetchData (2)
- Description: Data are finally fetched from the step 3
- Dispatch:
{
type: ACCOUNT_RECEIVE_DATA,
payload: { data }
}
7. Account Reducer
- Description: Reducers are notified of the dispatch and modifies their values accordingly
- Reducers:
{ isFetching: false, didInvalidate: false, data: { protectedString: '42: The answer to life' } }
8. Third render
- Description: Data are fetched and ready to be displayed
- Reducers:
{ isFetching: false, didInvalidate: false, data: { protectedString: '42: The answer to life' } }
- Render:
<div>42: The answer to life</div>
Hope it helps.
Edit: Let me answer your question in one of your comment in the other answer
@Icepickle I'm not sure it's a clean way to do that. Suppose the user will go to /account URL. Then to some other URL. Then back to the /account. While the data will be loading from the server for the second time, the isFetching will be true and the "loading" text must be shown, but the "data" variable will not be null, because it will contain the data from the previous request. So, instead of "loading", the old data will be shown.
With the didInvalidate
value, there is no risk of unlimited refetching as the component will know wether your data are valid or not.
In the componentDidMount
, the condition to refetch will be false as the values are the following { isFetching: false, didInvalidate: false }
. No refetch then.
if (this.props.didInvalidate && !this.props.isFetching)
Bonus: However be careful of data caching issues with the didInvalidate
.
People don't talk much about this issue but you will start asking this question "Starting when my data are invalid ?" (= When should I refetch ?)
Reducers
If I may, let me refactor your reducer code for the long run.
Your reducers will be way more modular and easy to maintain this way.
import { combineReducers } from 'redux';
export default combineReducers({
didInvalidate,
isFetching,
lastFetchDate,
data,
errors,
});
function lastFetchDate(state = true, action) {
switch (action.type) {
case 'ACCOUNT_RECEIVE_DATA':
return new Date();
default:
return state;
}
}
function didInvalidate(state = true, action) {
switch (action.type) {
case 'INVALIDATE_ACCOUNT':
return true;
case 'ACCOUNT_RECEIVE_DATA':
return false;
default:
return state;
}
}
function isFetching(state = false, action) {
switch (action.type) {
case 'ACCOUNT_FETCH_DATA_REQUEST':
return true;
case 'ACCOUNT_RECEIVE_DATA':
return false;
default:
return state;
}
}
function data(state = {}, action) {
switch (action.type) {
case 'ACCOUNT_RECEIVE_DATA':
return {
...state,
...action.payload.data,
};
default:
return state;
}
}
function errors(state = [], action) {
switch (action.type) {
case 'ACCOUNT_ERRORS':
return [
...state,
action.error,
];
case 'ACCOUNT_RECEIVE_DATA':
return state.length > 0 ? [] : state;
default:
return state;
}
}
Actions
I will just add the invalidation function so it will be easier to understand which function I call in the component.
(Note: I did not rename your functions but you should definitely pay attention at the naming)
export function invalidateAccount() {
return {
type: INVALIDATE_ACCOUNT
};
}
export function accountReceiveData(data) {
return {
type: ACCOUNT_RECEIVE_DATA,
payload: {
data
}
};
}
export function accountFetchDataRequest() {
return {
type: ACCOUNT_FETCH_DATA_REQUEST
};
}
export function accountFetchData(token) {
return (dispatch, state) => {
dispatch(accountFetchDataRequest());
axios({
// axios parameters to fetch data from the server
})
.then(checkHttpStatus)
.then((response) => {
dispatch(accountReceiveData(response.data));
})
.catch((error) => {
//error handling
});
};
}
Component
You will have to invalidate your data at some point. I considered that your account data would not be valid anymore after 60 minutes.
import isBefore from 'date-fns/is_before';
import addMinutes from 'date-fns/add_minutes'
const ACCOUNT_EXPIRATION_MINUTES = 60;
class AccountView extends React.Component {
componentDidMount() {
const token = this.props.token;
// fetching the data using credentials
if (this.props.didInvalidate && !this.props.isFetching) {
this.props.actions.accountFetchData(token);
}
// Below the check if your data is expired or not
if (
!this.props.didInvalidate && !this.props.isFetching &&
isBefore(
addMinutes(this.props.lastFetchDate, ACCOUNT_EXPIRATION_MINUTES), new Date()
)
) {
this.props.actions.invalidateAccount();
}
}
componentWillReceiveProps(nextProps) {
if (nextProps.didInvalidate && !nextProps.isFetching) {
nextProps.actions.accountFetchData(token);
}
}
render() {
const {
isFetching,
didInvalidate,
lastFetchDate,
data,
} = this.props;
/*
* Do not display the expired data, the componentDidMount will invalidate your data and refetch afterwars
*/
if (!didInvalidate && !isFetching &&
isBefore(addMinutes(lastFetchDate, ACCOUNT_EXPIRATION_MINUTES), new Date())
) {
return <Loading />;
}
if (isFetching || (didInvalidate && !isFetching)) {
return <Loading />; // You usually have your loading spinner or so in a component
}
return (
<div>
{data.protectedString}
</div>
);
}
}
This code can be cleaner but it is clearer to read that way :)