Open krista-koivisto opened 1 year ago
I'm curious if you have quantitative data about the overhead in this particular case. Could you add that if you do?
@charkour That is a very valid question.
Unfortunately the official React documentation doesn't go into more detail than suggesting it be used for "expensive" calculations. However, I did find an attempt at quantifying the performance. The main takeaways from that article is:
The real issue that I see with premature optimization is the poor developer experience resulting from it. The potential gain in processing time is not significant enough to offset the cost in developer time. That time can later be used to optimize where it truly is necessary.
Roger, thank you for the write-up! Looks like the maintainers aren't super active here, but I appreciate you raising concerns!
There is currently a fair bit of unnecessary
useMemo
use in the codebase. It creates a cluttery codebase that may be off-putting to would-be contributors while not actually leading to improved performance. It will even hurt performance due to the overhead of the hook in many cases.Here is an example of such a case: https://github.com/Stability-AI/StableStudio/blob/76772d7b9a4e6d87bd87c26665f7ab74cd40f2cd/packages/stablestudio-ui/src/Generation/Image/index.tsx#L153-L165
In this case, the overhead of
useMemo
is going to be significantly worse than letting React discover on its own that it doesn't need to rerender the component.useMemo
should ideally be used sparingly and for truly resource-intensive tasks, or in some special cases such as ensuring referential equality.It is also (almost always) a bad idea to wrap the return statements of functional components in
useMemo
as React is VERY good at knowing when it needs rerender components.I really think it would be good to clean this up if it is okay with the product owner(s). Doing so would lead to a friendlier codebase that may attract more contributors and would likely make StableStudio more optimized in the process.