React useEffect has a spread element in its dependency array
Asked Answered
B

4

7

I'm trying to build a custom hook that will handle loading and error behavior of an async call.

I'd like to use it like so :

const { loading, error} = useFetch(fetchUser, id)

or

const {loading, error } = useFetch(updateUserName, id, name)

I have something that look like this so far :

function useFetch(fetchFn, ...params) {
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState(false);

  useEffect(() => {
    fetchFn(...params)
      .then(() => {
        setLoading(false);
      })
      .catch(() => {
        setError(true);
      });
  }, [...params, fetchFn]);

  return { loading, error };
}

I'm having trouble with the useFetch(fetchFn, ...params) since a new params array is created on every call. So I cannot use it in the dependency array. But i can't use spread operator.

Is it possible to achieve what i'm trying to do ?

Bombard answered 29/11, 2019 at 15:47 Comment(4)
You want to fetch just once right? Or everytime one of the parameters changes?Trenttrento
@Dupocas, yes i'd like to fetch only once for a given set of params. ie: useFetch(fetchUser, id) or useFetch(updateUserName, id, name)Bombard
Could you post some real code of how you're calling useFetch? From where those parameters come from (state, props, static). Probably the solution here is to wrap fetchFn into an useCallback to ensure it's stable, and memoize params. But it's hard to guess without knowing the origin of paramsTrenttrento
I updated the question to show how i'd like to use the function. I've had the same issue with useCallback. I definitely need to memo the params but it's not clear to me how to.Bombard
B
3

EDIT: I now use react-query and my life is simpler

I managed to have the wanted behavior :

function useFetch(fetchFn, ...params) {
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState(false);

  useEffect(() => {
    fetchFn(...params)
      .then(() => {
        setLoading(false);
      })
      .catch(() => {
        setError(true);
      });
  }, params);

  return { loading, error };
}

I simply pass the params as the dependency array. Of course eslint isn't happy but we can simply disable it. I believe this implementation is correct.

Please feel free to share your opinions, i'm really interested on your feedbacks

Bombard answered 29/11, 2019 at 17:15 Comment(6)
This works. But please notice that you are lying about your dependencies. Be carefulTrenttrento
Official React typescript type of the dependency list is type "ReadonlyArray<any>" so it's not safe even if it works.Kiersten
What do you mean by 'it's not safe' ? params here are indeed a ReadonlyArray<any>Bombard
The way hooks such as useEffect work, is by managing correct dependencies. Giving wrong dependencies may lead to hard-to-debug bugs. That's why exhaustive-deps eslint rule exists. And that's so true in this code snippet you shared. a regular hook user would expect their fetchFn be called whenever it is changed.Sleepless
@ArdeshirIzadi, exhaustive-deps wants to add functions and other objects to the array, but when you do that you get an infinite loop, because these items are generated from scratch on every render and they aren't equal (===). Sometimes it's necessary to not follow it.Maddie
@Maddie The best practice would be to use useCallback & useMemo for every object/function that is vital to React's re/rendering mechanism. If the fetchFn is defined inside a React component, then it definitely is dependent either on a state or on a param. So the developer must expect the fetchFn to be updated (and re-called) when that prop/state changes, so it has to be included in the dependency array. If it is not dependent on any prop/state, then the fetchFn must be defined outside of React lifecycle, so it can be included inside the dependencies array. exhaustive-deps helps!Sleepless
A
0

Presumably you want the individual parameters to be the dependency and not an array of those parameters? Because in that case the dependency is really just the array instance (which as you say will change every time).

Don't spread the parameters in the dependency list, instead include those parameters in the dependency list i.e.

useEffect(() => {
  fetchFn(...params)
    .then(() => {
      setLoading(false);
    })
    .catch(() => {
      setError(true);
    });
}, [fetchFn].concat(params));
Althing answered 29/11, 2019 at 15:56 Comment(7)
This still isn't an array literal thus cannot be statically verifyTrenttrento
@Trenttrento can you elaborate? Based on the example usage given I'd expect the dependency list to be [fetchFn, id, name]. I've not tested this code yet but I am fairly certain that's what I would get. The rest parameter (params) should contain whatever other parameters are passed after fetchFn and concat joins an array to another. What am I missing here?Althing
The deps declaration must be an array literal, hard coded for lack of better words. If the deps array is a product of an operation (event something like [].concat([])) React can't verify it statically (since the result is only known at runtime). If you delcare it like in your example eslint will curse youTrenttrento
@Trenttrento is that definitely the case :/ ? Why would static verification matter in this instance? The arrays are checked by React on each run, I don't see why static analysis is relevant here, particularly when that's the whole point. I've never known this to be a problem.Althing
Here. A reproducible example codesandbox.io/s/… It's not an error in this case because the evalution will always be the same (but eslint still won't like it), but if any dynamic properties are included you can come across a situation where the dependencies from next render are different than the next one, in better words: Where the signatures from the deps array are different between renders)Trenttrento
@Trenttrento ah, the infamous exhaustive-deps warning, I understand what you mean now, thanks for clarifying. I think in hindsight really the simplest approach would be for the OP to name the exact parameters in the signature and not use the rest parameter (there doesn't appear to be too many expected).Althing
The problem with "manual" naming of params is fetch functions tend to be written as <Args = any[], ReturnValue = unknown>(...args: Args) => Promise<ReturnValue>. This makes them easier to use by only requiring the data in signature instead of full url. What react expects though is to write a separate hook for each such function to satisfy the static analysis, which goes against the idea of custom hooks in the first place. It also assumes url strings are passed as arguments instead of URL instances. And those url strings being absolute and not being joined somewhere down the line.Claiborn
M
0

This is a dynamic reusable fetcher that doesn't cause extra renders. dependencies is an object (name / value pairs essentially), which allows you to dynamically replace query variables. The code can be modified for any data source.

So dependencies therefore isn't a param array but an object. The function is just going to watch these dependencies and reload the query when they change

My template included strings like: ${myQueryVariable} which get replaced with their values.

import { useEffect, useState } from "react"

export function useQuery(gqlTemplate: string, valid: () => boolean, resolve: (data: any) => void, dependencies: any) {

    const [loading, setLoading] = useState(false)

    var dependencyList: string = JSON.stringify(dependencies)

    useEffect(() => {
        async function fetchData() {
            if (valid()) {
                setLoading(true)
                var some: string = Object
                    .keys(dependencies)
                    .reduce(
                        (acc, cur) => acc.replaceAll(new RegExp(`\\$\{${cur}\}`, 'g'), dependencies[cur].toString()),
                        gqlTemplate
                    )

                const response = await fetch(`/graphql?query=${some}`)
                const json = await response.json()
                resolve(json.data)
                setLoading(false)
            }
        }
        fetchData()
    }, [dependencyList])
    return [loading] as const
}

example use

  const [loading] = useQuery(
    myQueryTemplate,
    () => true,
    (data: any) => {
      setMyData(data)
    },
    { myQueryVariable }
  )
Maddie answered 8/2, 2023 at 15:19 Comment(0)
I
-3

How about doing this?

ATTENTION: this is not a runnable code. It even wasn't be transpiled. It lacks parts of code and should probably has a lots of typos. Use it as reference only.

/*
Firstly, let's define a function to determine if two arrays have the same elements.
*/
const hasSameValues = (arr1 = [], arr2 = []) => {
  if (arr1?.length !== arr2?.length) {
    return false;
  }

  for (let i = 0, length = arr1?.length; i < length; i++) {
    if (arr1[i] !== arr2[i]) {
      return false;
    }
  }
  return true;
}

/* 
 Now we define the compoenent that will receive the dependencies array
*/

const AircraftComboBox = ({source = () => {}, dependencies = [], onChange = () => {}}) => {
  
  // Let's save the dependencies to evaluate if they've changed in the next render
  const [storedDependencies, setStoredDependencies] = useState(dependencies);

  const [availableAircrafts, setAvailableAircrafts] = useState([]);

  useEffect(
    () => {
      // Use the hasSameValues function to determine in runtime if the array of dependecies has changed
      setStoredDependencies(prev => hasSameValues(prev, dependencies) ? prev : dependencies)
    },
    [dependencies, setStoredDependencies] // this array of dependencies still hardcoded
  );

  useEffect(() => {
    async function fetchData() {
      const response = await source(); // Call the source function passed in props
      setAvailableAircrafts([...response.data]);
    }
    
    fetchData();
  }, [storedDependencies]); // this array of dependencies still hardcoded


  return (
    <>
      <label for="aircrafts-combo">Aircraft</label>
      <select id="aircrafts-combo" onChange={e => onChange(e)}>
        { availableAircrafts.map(option => <option value={option}>{option}</option>) }
      </select>
    </>  
  );
}



/**
  Now we can use it.
  Note that the Component using AircraftComboBox doesn't need to implement any useEffect.
 */
 const AircraftFeature = () => {
   const [category, setCategory] = useState('');
   const [airCraft, setAircraft] = useState('');

   return (
     <>
      <div>
        <label for="category">Category</label>
        <select id="category" type="text" onChange={e => setCategory(e.target.value)} >
          <option value="ac">airplane</option>
          <option value="rc">rotorcraft</option>
          <option value="pl">powered lift</option>
          <option value="gl">glider</option>
          <option value="lta">lighter than air</option>
          <option value="pp">powered parachute</option>
          <option value="wsc">weight-shift-control</option>
        </select>  
      </div>

    {/**
     Every time category's change, source function, defined in props, will be fired.
     This will retrieve new data to populate the list of available options.
     */}

      <AircraftComboBox 
        source={() => httpService.getAirplanesByCategory(category)}
        dependencies={[category]}
        onChange={e => setAircraft(e.target.value)}
      /> 

    </>
   );
 }
Instantaneous answered 22/1, 2021 at 17:52 Comment(3)
Please don't post code answers if they have "lots of typos"Linnlinnaeus
Seriously, @RussJ? You know what a polite person means by the word "probably"? Probably no. You know... probably the code has no typos. Probably I am being too polite, more than you expect. Sorry for that.Instantaneous
@RussJ?, Sorry if I didn't donate enough time as you expected. To make sure there were no errors, I would have to implement an entire use case. If I did that it was better to host the content in some sandbox and not here. Right now I put a slightly different version of this in a project and it worked, but I can't put the code here, 'cause there are missing dependencies. This version can be adapted for anyone who wants to use it and evaluate it. I hope it works for someone who minimally wants to share a little effort. Now I understand why people prefer not to contributeInstantaneous

© 2022 - 2024 — McMap. All rights reserved.