Why the fetchData function is defined inside the useEffect and not outside ?

Link: .js/blob/canary/examples/with-graphql-faunadb/lib/useFetch.js

import { useState, useEffect } from 'react'

export default function useFetch(url, options) {
  const [data, setData] = useState(null)
  const [error, setError] = useState(null)

  useEffect(() => {
    const fetchData = async () => {
      try {
        const res = await fetch(url, options)
        const json = await res.json()

        setData(json)
      } catch (error) {
        setError(error)
      }
    }
    fetchData()
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [url])

  return { data, error }
}

I would have done that:

import { useState, useEffect } from 'react'

export default function useFetch(url, options) {
  const [data, setData] = useState(null)
  const [error, setError] = useState(null)

  // Defined outside of useEffect
  // `u` instead of `url` for not overlapping
  // with the one passed in useFetch()
  const fetchData = async (u) => {
    try {
      const res = await fetch(u, options)
      const json = await res.json()

      setData(json)
    } catch (error) 
      setError(error)
    }
  }

  useEffect(() => {
    // Using url as an argument
    fetchData(url)
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [url])

  return { data, error }
}

It seems easier to read and better organized. I'm thinking it's maybe an anti-pattern or something else ?

Why the fetchData function is defined inside the useEffect and not outside ?

Link: https://github.com/zeit/next.js/blob/canary/examples/with-graphql-faunadb/lib/useFetch.js

import { useState, useEffect } from 'react'

export default function useFetch(url, options) {
  const [data, setData] = useState(null)
  const [error, setError] = useState(null)

  useEffect(() => {
    const fetchData = async () => {
      try {
        const res = await fetch(url, options)
        const json = await res.json()

        setData(json)
      } catch (error) {
        setError(error)
      }
    }
    fetchData()
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [url])

  return { data, error }
}

I would have done that:

import { useState, useEffect } from 'react'

export default function useFetch(url, options) {
  const [data, setData] = useState(null)
  const [error, setError] = useState(null)

  // Defined outside of useEffect
  // `u` instead of `url` for not overlapping
  // with the one passed in useFetch()
  const fetchData = async (u) => {
    try {
      const res = await fetch(u, options)
      const json = await res.json()

      setData(json)
    } catch (error) 
      setError(error)
    }
  }

  useEffect(() => {
    // Using url as an argument
    fetchData(url)
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [url])

  return { data, error }
}

It seems easier to read and better organized. I'm thinking it's maybe an anti-pattern or something else ?

• javascript
• reactjs
• react-hooks
• next.js

• 1 I've found in the React docs my answer: fr.reactjs.org/docs/… – Adrien Commented Apr 30, 2020 at 16:52

2 Answers 2

I typically define the functions inside the useEffect, there are several reasons for that

1. By definining the function outside of the use effect, you either need to disable exhaustive-deps and risk accidentally having a stale function or you need to useCallback to make the function not-update every render
2. If the function is only used in the useEffect, you don't need to recreate the function on every render as that's just wasted cycles
3. It's easier to work with cleanup on the asynchronous functions by defining it within useEffect as you can define variables that are able to be modified within the effect.

On that last one, for instance, you can do some actions to prevent state being called when the effect cleans up.

You could also use AbortController with fetch to cancel the fetch.

import { useState, useEffect } from 'react'

export default function useFetch(url, options) {
  const [data, setData] = useState(null)
  const [error, setError] = useState(null)

  useEffect(() => {
    let isUnmounted = false;
    const fetchData = async () => {
      try {
        const res = await fetch(url, options)
        const json = await res.json()
        if(!isUnmounted) setData(json)
      } catch (error) {
        if(!isUnmounted) setError(error)
      }
    }
    fetchData()
    return ()=>{isUnmounted = true;}
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [url])

  return { data, error }
}

Technically fetchData should be a dependency for useEffect according to React Hooks rules. But if you add it, it will give you an error saying it will cause useEffect to run on every re-render as the function is recreated IF this hook is defined inside a component.

But since it is defined outside a component, my understanding is that the function will not be recreated. Then just add the fetchData as a dependency.

If this useEffect was used inside a component you could either just pass the function inside the useEffect or add the dependency and cover your fetchData with a useCallback.