4

In my react-typescript application, I am trying to use a context provider that encapsulates properties and methods and exposes them for a consumer:

const StockPriceConsumer: React.FC = () => {
  const stockPrice = useContext(myContext);
  let val = stockPrice.val;
  useEffect(() => {
    stockPrice.fetch();
  }, [val]);
  return <h1>{val}</h1>;
};

The problem is the following warning:

React Hook useEffect has a missing dependency: 'stockPrice'. Either include it or remove the dependency array. eslint(react-hooks/exhaustive-deps)

To me it does not make any sense to include the stockPrice (which is basically the provider's API) to the dependencies of useEffect. It only makes sense to include actual value of stock price to prevent infinite calls of useEffect's functions.

Question: Is there anything wrong with the approach I am trying to use or can I just ignore this warning?


The provider:

interface StockPrice {
  val: number;
  fetch: () => void;
}

const initialStockPrice = {val: NaN, fetch: () => {}};

type Action = {
  type: string;
  payload: any;
};

const stockPriceReducer = (state: StockPrice, action: Action): StockPrice => {
  if (action.type === 'fetch') {
    return {...state, val: action.payload};
  }
  return {...state};
};

const myContext = React.createContext<StockPrice>(initialStockPrice);

const StockPriceProvider: React.FC = ({children}) => {
  const [state, dispatch] = React.useReducer(stockPriceReducer, initialStockPrice);
  const contextVal  = {
    ...state,
    fetch: (): void => {
      setTimeout(() => {
        dispatch({type: 'fetch', payload: 200});
      }, 200);
    },
  };
  return <myContext.Provider value={contextVal}>{children}</myContext.Provider>;
};
Aprillion
  • 21,510
  • 5
  • 55
  • 89
sovo2014
  • 487
  • 7
  • 17
  • rule of thumb: the feeling that you want to ignore `react-hooks/exhaustive-deps` == your side effects are fighting against [Thinking in React](https://reactjs.org/docs/thinking-in-react.html) Step 4: Identify Where Your State Should Live – Aprillion May 23 '20 at 09:18

2 Answers2

1

I would recommend to control the whole fetching logic from the provider:

const StockPriceProvider = ({children}) => {
  const [price, setPrice] = React.useState(NaN);

  useEffect(() => {
    const fetchPrice = () => {
      window.fetch('http...')
       .then(response => response.json())
       .then(data => setPrice(data.price))
    }
    const intervalId = setInterval(fetchPrice, 200)
    return () => clearInterval(intervalId)
  }, [])

  return <myContext.Provider value={price}>{children}</myContext.Provider>;
};

const StockPriceConsumer = () => {
  const stockPrice = useContext(myContext);
  return <h1>{stockPrice}</h1>;
};

...as a solution to a couple of problems from the original appproach:

  1. do you really want to fetch only so long as val is different? if the stock price will be the same between 2 renders, the useEffect won't execute.
  2. do you need to create a new fetch method every time <StockPriceProvider> is rendered? That is not suitable for dependencies of useEffect indeed.

    • if both are OK, feel free to disable the eslint warning
    • if you want to keep fetching in 200ms intervals so long as the consumer is mounted:
  // StockPriceProvider
  ...
    fetch: useCallback(() => dispatch({type: 'fetch', payload: 200}), [])
  ...
  // StockPriceConsumer
  ...
    useEffect(() => {
      const i = setInterval(fetch, 200)
      return () => clearInterval(i)
    }, [fetch])
  ...
Aprillion
  • 21,510
  • 5
  • 55
  • 89
0

The important concept here is that react compares the objects by reference equality. Meaning that every time the reference (and not the content) changes it will trigger a re-render. As a rule of thumb, you always need to define objects/functions that you want to pass to child components by useCallback and useMemo.

So in your case: The fetch function will become:

const fetch = useCallback(() => {
    setTimeout(() => {
        dispatch({ type: 'fetch', payload: 200 });
    }, 1000);
}, []);

The empty array means that this function will be only defined when the component is mounted. And then:

let {val, fetch} = stockPrice;

 useEffect(() => {
    fetch();
 }, [val, fetch]);

This means the useEffect's callback will execute only when fetch or val changes. Since fetch will be defined only once, in practice it means only val changes are gonna trigger the effect's callback.

Also, I can imagine you want to trigger the fetch only when isNaN(val) so:

let {val, fetch} = stockPrice;

 useEffect(() => {
    if(isNaN(val)) {
        fetch();
    }
 }, [val, fetch]);

All that being said, there's a bigger issue with this code!

You should reconsider the way you use setTimeout since the callback can run when the component is already unmounted and that can lead to a different bug. In these cases you should useEffect and clear any async operation before unmounting the component. So here's my suggestion:

import React, { useCallback, useContext, useEffect } from 'react';
interface StockPrice {
    val: number;
    setFetched: () => void;
}

const initialStockPrice = { val: NaN, setFetched: () => { } };

type Action = {
    type: string;
    payload: any;
};

const stockPriceReducer = (state: StockPrice, action: Action): StockPrice => {
    if (action.type === 'fetch') {
        return { ...state, val: action.payload };
    }
    return { ...state };
};

const myContext = React.createContext<StockPrice>(initialStockPrice);

const StockPriceProvider: React.FC = ({ children }) => {
    const [state, dispatch] = React.useReducer(
        stockPriceReducer,
        initialStockPrice
    );
    const setFetched = useCallback(() => {
        dispatch({ type: 'fetch', payload: 200 });
    }, []);
    const contextVal = {
        ...state,
        setFetched,
    };
    return <myContext.Provider value={contextVal}>{children}</myContext.Provider>;
};

const StockPriceConsumer: React.FC = () => {

    const stockPrice = useContext(myContext);
    const {val, setFetched} = stockPrice;

    useEffect(() => {
        let handle = -1;
        if(isNaN(val)) {
            let handle = setTimeout(() => { // Or whatever async operation
                setFetched();
            }, 200);
        }
        return () => clearTimeout(handle); // Clear timeout before unmounting.
    }, [val, setFetched]);
    return <h1>{stockPrice.val.toString()}</h1>;
};