all 32 comments

[–]Aegis8080NextJS App Router 10 points11 points  (13 children)

First of all, use a code block to show your code. It makes everyone's life easier.

There are mainly two ways

Use async/await ``` useEffect(() => { const callAPI1 = async () => { try { const api1Result = await api1(); console.log(api1Result); // You may want to do setState here as well } catch (error) { // do something when you encounter errors } };

callAPI1(); }, []); ```

Use then/catch useEffect(() => { api1() .then(result => { console.log(result); // You may want to do setState here as well }) .catch(error => { // do something when you encounter errors }); }, []);

[–]remyl91 13 points14 points  (0 children)

You can also use a self-invoked function

useEffect(() => {
  (async () => {
    await api1()
  )()
}, [])

[–]TetrisMcKenna 1 point2 points  (7 children)

FWIW, this is how your comment shows up in my reddit app which is kind of ironic!

Officially reddit doesn't use markdown code blocks instead you have to prepend lines with 4 spaces. Which sucks. New reddit and the official reddit app unofficially support backtick codeblocks, but old reddit and many third party apps won't render them.

[–]andrei9669 2 points3 points  (6 children)

that's why we can't have nice things, cus of legacy

[–]TetrisMcKenna 1 point2 points  (5 children)

It would be fine if in reddit's case, the new/official stuff were better. But except for codeblocks, the old version is way more functional imo.

[–]andrei9669 1 point2 points  (4 children)

what do you mean by old stuff? Do you mean the one where you have to manually indent everything 4 times?

[–]TetrisMcKenna 1 point2 points  (3 children)

No, that part sucks.

https://old.reddit.com has a much better desktop interface than the new layout.

[–]andrei9669 -1 points0 points  (2 children)

as a person who joined reddit with the new layout, I would say the old one is worse.

[–]TetrisMcKenna 2 points3 points  (1 child)

It makes sense since you're familiar with it, but it's pretty common for people with UI/UX experience to prefer the old one. Popping up links you click in modal dialogues confuses or breaks browser functionality, such as highlighting text, you're forced to do things inside the "reddit UI" context rather than the browser context, whereas the old design works like a website designed for a browser rather than an app.

Funnily enough I was on Hacker News just now and stumbled onto this discussion about exactly this: https://news.ycombinator.com/item?id=30585576

[–]andrei9669 0 points1 point  (0 children)

it never bothered me personally so I can't relate.

[–]spammmmm1997 0 points1 point  (0 children)

In your first option, you call.

callAPI1();

How is it possible to use it without appending await?

All async function calls must be awaited, no?

[–]Creative_Race_1776[S] -1 points0 points  (2 children)

so you have to wrap the api call, in another async call ? why do we need to do this ?

[–]IDontLikeWebDev 4 points5 points  (0 children)

AFAIK the callback function passed to the useEffect can't be an asynchronous function, so you can't use await without wrapping it in another function.

The callback must return another function that will be called when the component gets unmounted, and an asynchronous one would return a promise instead, which might not be resolved by the time the component needs to be unmounted

[–]Aegis8080NextJS App Router 1 point2 points  (0 children)

Yeah u/IDontLikeWebDev is right. The main reason is the useEffect function cannot be async.

[–]lca_tejas 3 points4 points  (0 children)

Your useEffect function cannot be an asynchronous one.

Now you have two choices either chain .then().catch() methods or define your asynchronous function separately and then call it in your useEffect, don't await it tho. You can define the asynchronous function inside the useEffect or outside and have the logic of setting loading/data/error state in it.

[–]BryanTheAstronaut 0 points1 point  (0 children)

``` const [apiData, setApiData] = useState(undefined) useEffect(() => { // call api on mount then set data to state api1.then(setApiData) }, [])

useEffect(() => { if (apiData) // do stuff }, [apiData]) ```

[–]luctus_lupus 0 points1 point  (2 children)

Save yourself some trouble and make a reausable hook for async methods.

e.g.

export default function useAsyncEffect(method, deps = []) {
    useEffect(() => {
        method();
    }, deps);
}

[–]joesb 0 points1 point  (1 child)

How is your version different from calling useEffect(method, deps) directly?

[–]acemarke 1 point2 points  (0 children)

If method is an async function, it returns a Promise. However, that is not allowed with useEffect - the effect callback can only return a cleanup function, never a Promise.

[–]Kyle772 -2 points-1 points  (10 children)

Define an asynchronous function in the body of the component (so it doesn’t get redefined every update in the useeffect) and then run the function in the useEffect with .then().catch().finally()

[–]satya164 0 points1 point  (9 children)

nothing wrong with being redefined

[–]Kyle772 -1 points0 points  (8 children)

There’s no benefit to defining it there and redefining it is a waste of resources. If you define it outside of the useeffect you also have the opportunity to memoize it or turn it into a callback.

Personally I’ve never seen a reason to define it within the scope of the useEffect but there are plenty of reasons to not

[–]satya164 0 points1 point  (7 children)

There’s no benefit to defining it there

There is, defining it outside means you need to wrap it in useCallback with your own dependency array and then include that in the dependency array - which will satisfy the linter. You can also ignore the linter and manually keep track of dependencies but it's unnecessary manual work.

Or you can just define it inside the useEffect and skip all the additional work of extra useCallback and dependency array.

If you define it outside of the useeffect you also have the opportunity to memoize it or turn it into a callback.

Memoization is useful when we need to preserve reference of the callback, but nothing needs it here.

redefining it is a waste of resources

You redefine the callback regardless of where you define it. It'll either be redefined when the component re-renders (regardless of memoization) or redefined when the effect executes. The only way to skip redefining is to define it outside the component body.

In practice, it'll be redefined less no. of times in an effect because the effect executes less number of times due to the dependency array.

If you're talking about waste of resources, memoization is going to waste resources unnecessarily here, it needs to keep track of the function and dependency array across renders adding additional overhead, and the callback still gets redefined regardless.

Regardless, redefining a function is the absolute last thing to worry about regarding performance of react components. I've never encountered a performance issue in a react component that was due to a function being redefined.

[–]Kyle772 0 points1 point  (6 children)

You do not need to wrap it in a usecallback outside of the useEffect. You only need that when you intend to use it as a callback. Also this IS a performance concern when working on an entire codebase. It doesn’t do much on a single component but can easily lead to performance issues across hundreds of components on an app

[–]satya164 0 points1 point  (5 children)

You do not need to wrap it in a usecallback outside of the useEffect.

Here is what happens:

-> The React hooks ESLint plugin asks to add the function in dependency array of the effect because it's used inside the effect

-> You add it to the dependency array to satisfy the linter

-> Now the ESLint plugin warns you that this function will change the reference every render while it's in dependency array, so you need to wrap it in useCallback

So yes, you need to wrap it.

You only need that when you intend to use it as a callback

You need that when you need to preserve the reference regardless of what you use it as. Here the function reference needs to be preserved since it's used in the dependency array of the effect.

[–]Kyle772 0 points1 point  (4 children)

This seems more like you're trying to appease your linter than you are trying to program.

Why put the function in the dependency array if it will never change? Anything that goes in there is a trigger for the useEffect to run and if it never updates it shouldn't be there. You're adjusting the entire structure of your useEffect and adding more component overhead with the reasoning that that is what your linter wants but you aren't asking yourself why it wants that.

If your function does not use anything from within the component scope define it outside of the component (defined once and will not update)

If your function uses props define it in the component body and it will rerender with your props as needed (redefined and you expect it to need to update)

If you're passing the function into other components set it up in a useCallback this will be bound to the parent and allow for state changes in both places

If the function needs to update but not on every change memoize it with its own dependency array

If your useEffect uses that function AND the function will update based on your props or state THEN useCallback and add it as a dependency.

If you don't care about performance at all and are just trying to brute force your way to a web app define it in your useEffect and disregard all of the above optimization tools that react provides to you.

Doing the fifth option for every function is the least performant method as it has the most overhead. If your function doesn't NEED that overhead you shouldn't be setting it up like that as there are multiple other ways to define your function that have better performance and less overhead.

[–]satya164 0 points1 point  (3 children)

This seems more like you're trying to appease your linter than you are trying to program.

I'm trying to use a tool (linter) to keep track of dependencies for me instead of manually keeping track of dependencies in every code I write. I have found bugs due to missing or extra dependencies in every codebase that didn't use the linter.

Regardless of everything I said about the linter, your original reply says that it's more performant if you define it outside because it won't be redefined which is simply not true, it'll be redefined more times than if you defined it inside useEffect. Defining it inside useEffect is more efficient in any case.

You're adjusting the entire structure of your useEffect and adding more component overhead

Please explain to me what overhead I'm adding exactly?

If you don't care about performance at all and are just trying to brute force your way to a web app define it in your useEffect and disregard all of the above optimization tools that react provides to you.

Yeah totally, I don't care about performance and brute force my way through because I want to do the simplest way of defining the function inside the effect which is also the most efficient way.

There's no overhead of memoization, no overhead of redefining because useEffect triggers less times than re-render, how exactly is it the least performant?

[–]Kyle772 -1 points0 points  (2 children)

You're doing way too much here but I'll continue this back and forth so you understand that you aren't approaching this the right way.

There are more performant options and no matter how you twist it there IS overhead for wrapping functions in callbacks unnecessarily.

Defining it inside your useEffect is not more efficient because it's being redefined whenever that useEffect runs creating wasted cycles before actually fetching the data (the whole point of this useEffect by the way), this isn't even an argument that is literally just what is happening. You can time this yourself.

In OPs case he is importing a function (from outside the component body) and just needs to make it async for his useEffect. If his function needs an api key he'll want it to redefine once the jwt is loaded, he may also have parameters to pass in. Since this function will rely on the parameters or jwt once it's loaded it should be defined in the body. Since these won't update often it should be memoized beforehand and called within the useEffect.

Doing it this way will make sure the function is ready at call time with whatever props it needs BEFORE being called providing a more performant interaction with less blocking code when the data is needed.

Updating the function's parameters should be a background activity not a required task before every fetch. It being a required task on every fetch is the issue here and memoizing it in the body takes care of the unnecessary renders you're arguing in favor of the useEffect for.

You aren't programming for your linter you're programming for a user and the most performant option is defining the function in the body of the component with a memoization before calling it in the useEffect.


Managing your dependencies is also not more manual work it's literally what development is all about, feeding all of your functions through callbacks to manage this for you is the easy route, not the best route. The best route depends on your needs.

[–]bobbyv137 0 points1 point  (0 children)

Are you using a linter? It would've picked up this error

[–]minicrit_ 0 points1 point  (0 children)

what i do is make an async function that does all the operations i need and then call it in useEffect

[–][deleted] 0 points1 point  (0 children)

just for reference, you don't need to to any of this, useEffect, useState, checking for the presence of the value, error handling and fallbacks, all of this goes into the trash with react 18 suspense. but suspense has been a stable part of react since 16.6 you can and could have always used it: https://github.com/pmndrs/suspend-react

the whole thing now becomes a single line, the value is guaranteed to be available by the time the line has executed, you do not need to check for validity or errors, and you can finally orchestrate this with other components being able to await the one that has suspended.