Not using useCallback is premature optimization(jnystad.no)
jnystad.no
Not using useCallback is premature optimization
https://jnystad.no/post/not-using-usecallback-is-premature-optimization
23 comments
For those pushing back, I wanted to share this talk given at a recent React Conf: https://www.youtube.com/watch?v=lGEMwh32soc
The idea (still just an internal prototype) is that one day your compiler might automatically memoize all React components and intermediate values for you. It obviously would be great to skip all the syntax noise, as well as the linter-enforced rules around correctly maintaining dependency arrays, etc.
But I think a big takeaway from this talk is that the React team sees memoizing as the norm. It is something to do by default, so much so that it may happen without your intervention one day. Immutability and value identity-based comparison are core to React. Violating these is the only way memoization can result in bugs (as some in this thread are worried about), and if you aren't following them then a lot of your React code is only working by accident to begin with, memo or no memo.
The idea (still just an internal prototype) is that one day your compiler might automatically memoize all React components and intermediate values for you. It obviously would be great to skip all the syntax noise, as well as the linter-enforced rules around correctly maintaining dependency arrays, etc.
But I think a big takeaway from this talk is that the React team sees memoizing as the norm. It is something to do by default, so much so that it may happen without your intervention one day. Immutability and value identity-based comparison are core to React. Violating these is the only way memoization can result in bugs (as some in this thread are worried about), and if you aren't following them then a lot of your React code is only working by accident to begin with, memo or no memo.
If React does it for me then fine, it's welcome. Until then why would I want to wrap all my functions in a bigger function?
How about we just do this:
import {useObey} from 'react'
// you may see no reason why you need to do this but just do it, not doing what you don't understand is obviously a fool's move.. btw.
const goodFn = useObey(myFn)
^// you'd better do this!
In OP's same article they are saying 'Senior' is just a title. Yeah juniors for sure will add an additional wrapper if you tell them. They'll add a useCallback around their useCallback for good measure. Part of being junior is just saying 'yes, mmhmm' to every thing that the hype train tells them to. I guess they are the real seniors since they listen so well.
You people pushing people to put all their callbacks in wrappers even if there is no discernible reason to, frankly suck and you're really sending the completely wrong message here. As a coder you should feel free to admit when you don't understand why something is told to you, and you should be comfortable not obeying the people who insist you need to just trust in their magic.
How about we just do this:
import {useObey} from 'react'
// you may see no reason why you need to do this but just do it, not doing what you don't understand is obviously a fool's move.. btw.
const goodFn = useObey(myFn)
^// you'd better do this!
In OP's same article they are saying 'Senior' is just a title. Yeah juniors for sure will add an additional wrapper if you tell them. They'll add a useCallback around their useCallback for good measure. Part of being junior is just saying 'yes, mmhmm' to every thing that the hype train tells them to. I guess they are the real seniors since they listen so well.
You people pushing people to put all their callbacks in wrappers even if there is no discernible reason to, frankly suck and you're really sending the completely wrong message here. As a coder you should feel free to admit when you don't understand why something is told to you, and you should be comfortable not obeying the people who insist you need to just trust in their magic.
My org just got done investigating the advantages and disadvantages of memoizing by default, and we decided to make it a default policy, because for a large org it's better to have a recommended best-practice that's going to make things perform better more often than not. Devs are still free to do otherwise if they profile and find a case where it causes harm, but frankly I don't see that happening.
The appeal to authority in my original comment is not just deferring to trends or whatever; it's leaning on the deep knowledge (and inside-knowledge!) that a core React team member has of not only React's current implementation, but its underlying intents/philosophy (which informs which paths the team will focus on optimizing, for example), and even its future roadmap. I think it's legitimate to take those things into account, especially in the absence of any significant, concrete evidence showing that the practice in question can cause problems.
The appeal to authority in my original comment is not just deferring to trends or whatever; it's leaning on the deep knowledge (and inside-knowledge!) that a core React team member has of not only React's current implementation, but its underlying intents/philosophy (which informs which paths the team will focus on optimizing, for example), and even its future roadmap. I think it's legitimate to take those things into account, especially in the absence of any significant, concrete evidence showing that the practice in question can cause problems.
> Devs are still free to do otherwise if they profile and find a case where it causes harm, but frankly I don't see that happening.
> I think it's legitimate to take those things into account, especially in the absence of any significant, concrete evidence showing that the practice in question can cause problems.
To me you are switching the burden of proof around from what it should be. What evidence do you have of the significant, concrete variety that wrapping every callback with useCallback, even if the deps array is empty, boosts performance?
> I think it's legitimate to take those things into account, especially in the absence of any significant, concrete evidence showing that the practice in question can cause problems.
To me you are switching the burden of proof around from what it should be. What evidence do you have of the significant, concrete variety that wrapping every callback with useCallback, even if the deps array is empty, boosts performance?
Re-rendering JSX subtrees is the main performance bottleneck in the majority of React apps. I should add that our decision includes the decision to start adding React.memo() to all components; otherwise, yeah, the benefits in most cases would be pretty small. But with React.memo(), useMemo/useCallback makes our components dramatically less likely to re-render, which again is the single biggest thing you can do for performance in most React apps. The difference is usually not perceptible on high-end dev machines, but a significant portion of our target market uses lower-end mobile devices to visit our site, so being performant-by-default is valuable to us.
Following this articles advice will likely make your app render slower than not optimizing at all. Caching everything increases your risk of stale data through misconfiguration. Also the benefits of referential stability only apply to a limited set of components in your overall render tree.
Most data structures and components don't need stability. The issue people have with hooks is primarily caused by 99% of folks not quite understanding just how sparingly hooks should be used.
You should only worry about referential equality once you've profiled a performance issue from some high frequency interaction. This is the only time to useCallback, useMemo, or any form of referential caching.
An element walks into a bar. If your component accepts children or any react elements as props, quite normal, all optimizations around referential equality for its inputs will be flung out the window as that component de-optimizes at every optimization prematurely added.
Another big mistake I see folks of all levels make often is expecting more program design upfront vs. say just managing configurations.
- <Example><Test /></Example>
- <Example => {}} />
- <Example test={{}} />
- <Example tests={[]} />
These all have the same render cost, so there is often no reason to extract function handlers/arrays/objects out of the jsx configuration.
Most data structures and components don't need stability. The issue people have with hooks is primarily caused by 99% of folks not quite understanding just how sparingly hooks should be used.
You should only worry about referential equality once you've profiled a performance issue from some high frequency interaction. This is the only time to useCallback, useMemo, or any form of referential caching.
An element walks into a bar. If your component accepts children or any react elements as props, quite normal, all optimizations around referential equality for its inputs will be flung out the window as that component de-optimizes at every optimization prematurely added.
Another big mistake I see folks of all levels make often is expecting more program design upfront vs. say just managing configurations.
- <Example><Test /></Example>
- <Example => {}} />
- <Example test={{}} />
- <Example tests={[]} />
These all have the same render cost, so there is often no reason to extract function handlers/arrays/objects out of the jsx configuration.
> Following this articles advice will likely make your app render slower than not optimizing at all.
Yes, but it will not actually matter in >99% of real world cases. And this argument is exactly what the article mentions, premature optimization.
> Caching everything increases your risk of stale data through misconfiguration.
Yes, but doing the opposite increases the risk of useEffects triggering unnecessarily (and wildly).
> The issue people have with hooks is primarily caused by 99% of folks not quite understanding just how sparingly hooks should be used.
I strongly disagree. The consequences of not using it by default, then arriving at a case where it matters, are so much bigger than the minor performance impact using it "everywhere" has.
But you need to have experienced this in a large and complex application to really feel it, I suppose.
Yes, but it will not actually matter in >99% of real world cases. And this argument is exactly what the article mentions, premature optimization.
> Caching everything increases your risk of stale data through misconfiguration.
Yes, but doing the opposite increases the risk of useEffects triggering unnecessarily (and wildly).
> The issue people have with hooks is primarily caused by 99% of folks not quite understanding just how sparingly hooks should be used.
I strongly disagree. The consequences of not using it by default, then arriving at a case where it matters, are so much bigger than the minor performance impact using it "everywhere" has.
But you need to have experienced this in a large and complex application to really feel it, I suppose.
> Yes, but it will not actually matter in >99% of real world cases. And this argument is exactly what the article mentions, premature optimization.
The author talks about non of the downsides of their approach. They are real.
> Yes, but doing the opposite increases the risk of useEffects triggering unnecessarily (and wildly).
useEffect in your component should be rare. Effects should not come from watching state/props. Effects should come from interactions.
> I strongly disagree. The consequences of not using it by default, then arriving at a case where it matters, are so much bigger than the minor performance impact using it "everywhere" has.
Explains why your viewpoint currently aligns with the author. I can tell from what you've said so far you use useEffect too much, which causes you to rely on referential equality more.
> But you need to have experienced this in a large and complex application to really feel it, I suppose.
Assume I have?
The author talks about non of the downsides of their approach. They are real.
> Yes, but doing the opposite increases the risk of useEffects triggering unnecessarily (and wildly).
useEffect in your component should be rare. Effects should not come from watching state/props. Effects should come from interactions.
> I strongly disagree. The consequences of not using it by default, then arriving at a case where it matters, are so much bigger than the minor performance impact using it "everywhere" has.
Explains why your viewpoint currently aligns with the author. I can tell from what you've said so far you use useEffect too much, which causes you to rely on referential equality more.
> But you need to have experienced this in a large and complex application to really feel it, I suppose.
Assume I have?
> The author talks about non of the downsides of their approach
The whole article is a counterpoint to a common falsehood. The downsides have been overstated a lot already.
> useEffect in your component should be rare.
Route is state too. Loading data based on route is very common. There is no reason to avoid useEffect for triggering side effects based on state or props in components. It's the whole reason it exists...
Or do you only use libraries for loading data, without realizing they rely on useEffect, perhaps?
> I can tell from what you've said so far you use useEffect too much
And I can tell from what you've said that you have fairly limited experience with React, or have not developed a variety of web applications with it.
> Assume I have?
Your statements make that difficult.
The whole article is a counterpoint to a common falsehood. The downsides have been overstated a lot already.
> useEffect in your component should be rare.
Route is state too. Loading data based on route is very common. There is no reason to avoid useEffect for triggering side effects based on state or props in components. It's the whole reason it exists...
Or do you only use libraries for loading data, without realizing they rely on useEffect, perhaps?
> I can tell from what you've said so far you use useEffect too much
And I can tell from what you've said that you have fairly limited experience with React, or have not developed a variety of web applications with it.
> Assume I have?
Your statements make that difficult.
Oh glad you responded. I'm happy to keep going for the sake of alignment and maybe we'll both find something out of this and the community will have more alignment.
> Route is state too. Loading data based on route is very common. There is no reason to avoid useEffect for triggering side effects based on state or props in components. It's the whole reason it exists... > Or do you only use libraries for loading data, without realizing they rely on useEffect, perhaps?
I've worked with many teams through this specific issue effectively inventing useEffect many times over before hooks. Loading a document or history change are exactly the types of interactions I was referring to in my comment. Absolutely handle this close to your routing/cache and you won't have to plumb stable objects to deep side-effects in your application.
> Your statements make that difficult.
Statements are hard to see if you don't have empathy for why folks are saying what they are saying it. From what I understand the author's points seems to revolve around their observation that react engineer's don't know things they should. While I certainly think that is true, in my experience I haven't professionally worked with any react engineers that aligned with any of the author's observation for sr/jr engineers.
I'm curious what your take is on the following:
> Some might also know useRef if they at some point tried to implement some DOM manipulation stuff (more often than not copied from SO) that probably should have been solved in CSS or otherwise. Most sr/jr people not familiar expect react to work like jquery or need an escape hatch to integrate with some non react lib that's expecting a mount point. In rare cases folks use refs for every element, like jquery, and just can't quite grasp react.
> useEffect and useState combo for processing some data without any side effects I've never seen this recommended and am not even sure what it means. Anything derived from useEffect is a side effect. I've seen this labeled "derived state" and folks often avoid it.
> One common pattern is the useEffect and useState combo for processing some data without any side effects, causing unnecessary render cycles between data change and computed value update. I've mostly seen useMemo/useRef/useSelectors used for this. I do agree with author that derived state should proxy to the underlying state and not copy it.
> useCallback and useMemo? Not so much. Everyone seems to know this. Jr and Sr folks use it all the time, even scenarios where they de-optimize
> Another is lying in the dependency arrays (and disabling that annoying lint rule) for various hooks since it was seemingly the only way to stop that useEffect from triggering the rate limiter of that API you were testing (we all do this from time to time, but removing the dependency is not the right solution). Never seen this. Maybe forget to add something to the array because they relied too much on react's linter doing this for them.
> Forget about "doing too much work", the primary reason useCallback and useMemo are your friends (and sometimes, useRef) is because they make those function, array and object references stable, and stable references work very well in dependency arrays. I've never experienced anyone using useCallback, useMemo, or useRef to prevent "doing too much work". Each one has been used for their specific use case with maybe useRef filling in for instance properties that don't trigger any effects.
The only other instance of these hooks i've seen is where useCallback and useMemo used is unnecessarily to keep objects stable in scenarios it didn't quite impact anything downstream. Another variation is where useCallback and useMemo are never stable as intended because the keys aren't stable to begin with. This distinction matters where sr engineers, yes sr, will easily accidentally do something that is effectively useCallback(handle, [Date.now()]).
> But why are these so unknown among so many that seemingly should know better? I guess most people don't really read the docs (breaking news). I'm pretty sure I've read every version of react's docs since launch and I guess i'm making the same mistakes. Also many of my team members who make constant mistakes with memoization are in the docs daily. I don't think everyone is missing the details you think they are.
> Route is state too. Loading data based on route is very common. There is no reason to avoid useEffect for triggering side effects based on state or props in components. It's the whole reason it exists... > Or do you only use libraries for loading data, without realizing they rely on useEffect, perhaps?
I've worked with many teams through this specific issue effectively inventing useEffect many times over before hooks. Loading a document or history change are exactly the types of interactions I was referring to in my comment. Absolutely handle this close to your routing/cache and you won't have to plumb stable objects to deep side-effects in your application.
> Your statements make that difficult.
Statements are hard to see if you don't have empathy for why folks are saying what they are saying it. From what I understand the author's points seems to revolve around their observation that react engineer's don't know things they should. While I certainly think that is true, in my experience I haven't professionally worked with any react engineers that aligned with any of the author's observation for sr/jr engineers.
I'm curious what your take is on the following:
> Some might also know useRef if they at some point tried to implement some DOM manipulation stuff (more often than not copied from SO) that probably should have been solved in CSS or otherwise. Most sr/jr people not familiar expect react to work like jquery or need an escape hatch to integrate with some non react lib that's expecting a mount point. In rare cases folks use refs for every element, like jquery, and just can't quite grasp react.
> useEffect and useState combo for processing some data without any side effects I've never seen this recommended and am not even sure what it means. Anything derived from useEffect is a side effect. I've seen this labeled "derived state" and folks often avoid it.
> One common pattern is the useEffect and useState combo for processing some data without any side effects, causing unnecessary render cycles between data change and computed value update. I've mostly seen useMemo/useRef/useSelectors used for this. I do agree with author that derived state should proxy to the underlying state and not copy it.
> useCallback and useMemo? Not so much. Everyone seems to know this. Jr and Sr folks use it all the time, even scenarios where they de-optimize
> Another is lying in the dependency arrays (and disabling that annoying lint rule) for various hooks since it was seemingly the only way to stop that useEffect from triggering the rate limiter of that API you were testing (we all do this from time to time, but removing the dependency is not the right solution). Never seen this. Maybe forget to add something to the array because they relied too much on react's linter doing this for them.
> Forget about "doing too much work", the primary reason useCallback and useMemo are your friends (and sometimes, useRef) is because they make those function, array and object references stable, and stable references work very well in dependency arrays. I've never experienced anyone using useCallback, useMemo, or useRef to prevent "doing too much work". Each one has been used for their specific use case with maybe useRef filling in for instance properties that don't trigger any effects.
The only other instance of these hooks i've seen is where useCallback and useMemo used is unnecessarily to keep objects stable in scenarios it didn't quite impact anything downstream. Another variation is where useCallback and useMemo are never stable as intended because the keys aren't stable to begin with. This distinction matters where sr engineers, yes sr, will easily accidentally do something that is effectively useCallback(handle, [Date.now()]).
> But why are these so unknown among so many that seemingly should know better? I guess most people don't really read the docs (breaking news). I'm pretty sure I've read every version of react's docs since launch and I guess i'm making the same mistakes. Also many of my team members who make constant mistakes with memoization are in the docs daily. I don't think everyone is missing the details you think they are.
I disagree with the author (and stated as much as recently as yesterday: https://news.ycombinator.com/item?id=32485460#32489682).
> With thousands of geometries in an interactive map, with lists and tables of items filtered by bounding box and categories, sorted by geodetic distance from wherever you were interested in, the performance issues that arose were never too many uses of useCallback or useMemo. The opposite caused issues multiple times however, of the app running haywire (or to a halt) kind.
The author here is taking their very valid and necessary use case for the optimization of memoization - non-trivial and expensive operations - and extrapolating from that to "just use useMemo, it's a best practice". Memoized functions are more complicated to reason about and therefore more bug prone than their alternative. Most client-side apps don't have thousands of records they're iterating over, they have dozens. Throwing around memo because one might one day become big, and you'd pay some performance penalty, doesn't seem prudent to me.
> With thousands of geometries in an interactive map, with lists and tables of items filtered by bounding box and categories, sorted by geodetic distance from wherever you were interested in, the performance issues that arose were never too many uses of useCallback or useMemo. The opposite caused issues multiple times however, of the app running haywire (or to a halt) kind.
The author here is taking their very valid and necessary use case for the optimization of memoization - non-trivial and expensive operations - and extrapolating from that to "just use useMemo, it's a best practice". Memoized functions are more complicated to reason about and therefore more bug prone than their alternative. Most client-side apps don't have thousands of records they're iterating over, they have dozens. Throwing around memo because one might one day become big, and you'd pay some performance penalty, doesn't seem prudent to me.
> The author here is taking their very valid and necessary use case for the optimization of memoization - non-trivial and expensive operations - and extrapolating from that to "just use useMemo
I would say that was your extrapolation, also it never says use useMemo everywhere.
You mention memo, which is different from useMemo in React, so I'm not sure if you are conflating them.
Either way, the main point is using useCallback by default, and using useMemo for non-trivial computed values. This will help you when your app grows in complexity.
If you understand hooks and why react triggers renders, you probably know when useCallback would not be necessary. However, most don't, and tend to err on the wrong side. Then they end up in hook hell when some component 6 levels deep need to use that callback function in a useEffect.
I would say that was your extrapolation, also it never says use useMemo everywhere.
You mention memo, which is different from useMemo in React, so I'm not sure if you are conflating them.
Either way, the main point is using useCallback by default, and using useMemo for non-trivial computed values. This will help you when your app grows in complexity.
If you understand hooks and why react triggers renders, you probably know when useCallback would not be necessary. However, most don't, and tend to err on the wrong side. Then they end up in hook hell when some component 6 levels deep need to use that callback function in a useEffect.
As a long, long time user of React (from back in the days before they even had a decent state management system and we were using libraries like "MartyJS" to handle state) I do feel like React has lost its way a little bit for regular users.
What benefits are the hooks providing over class based components to the average user?. I understand Facebook probably has very unique requirements around performance but 99% of people using the framework probably don't suffer from these problems but have to put up with the downsides of the occasional weirdness that hooks introduce.
Example, the way you have to handle useCallback is something that bit me very recently. I had a parent component tracking deleted gallery items as a map of ids and a boolean indicating if the image had been deleted. For some reason I couldn't figure out why the deletion map was always stale in the callback. Sure I had a useCallback around it but I'd correctly added the state variable in the dependency array of the useCallback hook.
Well, it turns out I need to pass the deletion map down to the child <Image /> components of the gallery where the deletion was called from because they also had a useCallback in them. The child image components have to know about all the images that have been deleted so that the parent component has the correct up to date state. Crazy!
I get that it's Facebooks project and they can do what they want with it, but I have 6+ years sunk into using the framework now, it's a lot to throw away. That said, I have high hopes for the combination of LiveView and Web Component based libraries like https://shoelace.style/
What benefits are the hooks providing over class based components to the average user?. I understand Facebook probably has very unique requirements around performance but 99% of people using the framework probably don't suffer from these problems but have to put up with the downsides of the occasional weirdness that hooks introduce.
Example, the way you have to handle useCallback is something that bit me very recently. I had a parent component tracking deleted gallery items as a map of ids and a boolean indicating if the image had been deleted. For some reason I couldn't figure out why the deletion map was always stale in the callback. Sure I had a useCallback around it but I'd correctly added the state variable in the dependency array of the useCallback hook.
Well, it turns out I need to pass the deletion map down to the child <Image /> components of the gallery where the deletion was called from because they also had a useCallback in them. The child image components have to know about all the images that have been deleted so that the parent component has the correct up to date state. Crazy!
I get that it's Facebooks project and they can do what they want with it, but I have 6+ years sunk into using the framework now, it's a lot to throw away. That said, I have high hopes for the combination of LiveView and Web Component based libraries like https://shoelace.style/
I would say the benefit of hooks and function based components is simplicity and ease-of-creation. Less boilerplate, less chance to mess up the life cycle of a component, especially for the vast majority of components that end up being pretty simple anyway.
I use hooks for animation, which requires setting up and tearing it down. I can do that with an easy hook, while if I had class based components, I'd have to keep track of every single time I want to use an animation, which is duplicated effort. Now multiply that by many different types of hooks, like querying for state, using a network connection (and not forgetting to close it down), etc.
Do you have a reduced code sample? I can’t quite picture the stale state issue.
(I suppose this is a good example in favour of the opposing “useCallback is premature optimisation” opinion)
(I suppose this is a good example in favour of the opposing “useCallback is premature optimisation” opinion)
This is the exact footgun I ran into yesterday:
https://codepen.io/rodeoclash/pen/poLqeYb?editors=1111
Even though `theState` is passed to the dependency array of the parent component which is tracking hidden images, it will still be stale!
After a while, I figured out that the child components, where the click originates from, also needed to have the map of deleted images passed to their dependency array. Total footgun.
https://codepen.io/rodeoclash/pen/poLqeYb?editors=1111
Even though `theState` is passed to the dependency array of the parent component which is tracking hidden images, it will still be stale!
After a while, I figured out that the child components, where the click originates from, also needed to have the map of deleted images passed to their dependency array. Total footgun.
For this specific example the problem is that the handleClick callback is using a stale/cached onImageDeleted callback (which indirectly has old state). If you add onImageDeleted to the handleClick dependency array, it beings to work as you expect.
Ahh, that's interesting.
Not on hand, but I'll put together one in a codepen now.
Having reliable 99% straight shots that encompass a variety of cases is so preferrable. I've been trying to advocate for consistent, low-control-flow decision making for so long, & this feels like what should have been an easy win example for me to make the case on. Im glad to see the topic/example come up again, with a much better specific elaboration.