dispatching actions on every render

Question

I am dispatching action addProducts on every mount of the ProductList component whereas i want to dispatch the action one timeusing useEffect hook and store the data in the redux and then use it.

Below are my actions file and ProductList component file.

actions.js file

export const addProducts = () => async (dispatch) => {
  let Products = await axios.get("https://api.npoint.io/2a4561b816e5b6d00894");
  return dispatch({
    type: ADD_PRODUCTS,
    payload: Products.data,
  });
};

ProductList.js component file

import { addProducts } from "../actions/Index";
const ProductList = () => {
  const dispatch = useDispatch();
  useEffect(() => {
   dispatch(addProducts()); 
  },[]);
  const Products = useSelector((state) => state.products);
  console.log(Products)

HMR · Accepted Answer · 2020-11-18 10:25:14Z

2

You could just dispatch the action in the component but in the thunk action do nothing if products are available:

export const addProducts = () => async (
  dispatch,
  getState,//thunk also get a getState function
) => {
  //you should write a dedicated selector function so you could do:
  //  const productsInState = selectProducts(getState())
  const productsInState = getState().products
  //whatever would make state.products available
  //  reducers are missing in your question
  if(productsInState){
    //do nothing if products are already in state
    return;
  }
  let Products = await axios.get(
    'https://api.npoint.io/2a4561b816e5b6d00894',
  );
  return dispatch({
    type: ADD_PRODUCTS,
    payload: Products.data,
  });
};

In your component you can just dispatch on each render, if your page has multiple components dispatching this action then you could make a grouped action.

answered Nov 18, 2020 at 10:25

HMR

39.5k26 gold badges99 silver badges175 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Hend El-Sahli Over a year ago

You don't know how long the axios-api-call would take ... during that time products in state will be null ... so their could be multiple components firing the same action causing multiple calls for axios.get .. ...

HMR Over a year ago

@HendEl-Sahli Ad mentioned in the answer; group the action if it can be dispatched within one render cycle.

peter554 · Accepted Answer · 2020-11-18 10:37:57Z

2

You want to check if products are already in redux with an if(!products){...} e.g.

const addProducts = () => async (dispatch) => {
  let Products = await axios.get("https://api.npoint.io/2a4561b816e5b6d00894");
  return dispatch({
    type: ADD_PRODUCTS,
    payload: Products.data,
  });
};

const ProductList = () => {
  const dispatch = useDispatch();
  const products = useSelector((state) => state.products);
  useEffect(() => {
   if (!products) {
     dispatch(addProducts()); 
   }
  },[dispatch]);
  return <p>foo</p>
}

edited Nov 18, 2020 at 10:37

answered Nov 18, 2020 at 10:25

peter554

1,3881 gold badge12 silver badges25 bronze badges

5 Comments

HMR Over a year ago

Why burdon the the component with the logic, this should be in the action.

peter554 Over a year ago

> this should be in the action. I don't think it really matters much, it could but I'm wouldn't go as far as should. Either way it answers the users question.

HMR Over a year ago

Then you don't understand redux. An action indicates something happened in this case component needs data if the action would fetch or get from cache is not something you should put in the component. If you do need to do this then pass cache=true or something to the action creator. If you change the way your cache works at a later time you need to change all code that dispatches the action instead of just change the one action.

peter554 Over a year ago

Well I like your enthusiasm and I see your point, but it really doesn't change much in terms of answering the users question.

HMR Over a year ago

The answer also has the problem Hend pointed out, if one render cycle dispatches that action multiple times you still fetch multiple times. So now you have multiple components that need some extra logic in the effect that all need to change do you see the point I'm trying to make? Working does not mean it is easy to maintain.

Hend El-Sahli · Accepted Answer · 2020-11-18 10:23:18Z

1

export const addProducts = () => async dispatch => {
  dispatch({
    type: ADD_PRODUCTS_START,
    payload: { loading: true },
  });

  const Products = await axios.get(
    'https://api.npoint.io/2a4561b816e5b6d00894'
  );

  dispatch({
    type: ADD_PRODUCTS_SUCCESS,
    payload: { products: Products.data, loading: false },
  });
};

const ProductList = ({ products, loading }) => {
  useEffect(() => {
    if (!products && !loading) {
      dispatch(addProducts());
    }
  }, []);
};


const mapStateToProps = ({ products: { data, loading } }) => ({ products: data, loading });

edited Nov 18, 2020 at 10:23

answered Nov 18, 2020 at 10:17

Hend El-Sahli

6,8685 gold badges31 silver badges50 bronze badges

2 Comments

HMR Over a year ago

You should not add this logic in the component, component should just dispatch the action, the action can decide if it wants to fetch products or not. At best you can pass cache=true to the action creator if you want your component to indicate it needs fresh data or not.

Hend El-Sahli Over a year ago

@HMR I must admit ... your answer is alot straight-farward .. and clean!

Collectives™ on Stack Overflow

dispatching actions on every render

3 Answers 3

2 Comments

5 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

5 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related