I've been writing functional React components for several years now. And while I feel that my code usually stands up to peer scrutiny, I've only r...
Some comments have been hidden by the post's author - find out more
For further actions, you may consider blocking this person and/or reporting abuse
From the top of my head:
Because memoization has a cost too. Unless there's something expansive in this component, you're gonna lose more performance to memoization than simply re-rendering the component as-is... If the component re-renders too many times, there's a problem upward the chain, not in this component
Because that's very bad code. I don't see people creating state they don't need unless they're really poor devs, in which case no amount of good practice is gonna save them
Recreating the object for styling isn't a problem per se, you're targeting an optimization that would shave maybe 5 nanoseconds... But, using inline-styles is really bad for browser performance which is actually mesurable with performance tools. Not to mention that inline styles can be high-jacked by malicious scripts and you should always block them with a Content Security Policy. You're 1000% better to import a CSS file and use classname instead. Not only does it solve the inline style performance, but it also makes your component reusable between projects.
Again, unless you NEED to optimize, simply don't. You won't gain any advantage by using
useCallback
or other performance enhancing tools. Memoization has a cost that won't be alleviated by these helper functions unless you do something expansive, which is relatively rare...This is not a performance problem, but a bad coding practice. Even if you replaced the inline function with the function reference, performance is not gonna improve even 1 nano second... I totally agree with you that this is totally horrible syntax and show the dev has not understood how to pass functions around, but it doesn't impact your performance in the slightest in the end.
One of the problems is that so many examples online and in documentation show things that use these patterns - probably because they are demonstrating a different point and doing it succinctly. Of course it then ends up looking "like best practice" when it patently isn't.
Side note, and not relevant to the points you are making here, but the game programmer in me silently screams when I code React, because I'm constantly making throw away things even if I AM avoiding the pitfalls you outline. (The angel on my right shoulder is always bellowing GC AIN'T FREE - into my ear).
There's also a huge lack of best-practices for large react apps. It's always very contained, small examples, simple functionality. At best, there's article #374 on which folder structure to use, or the thirteenth video on how to hyperoptimize a Counter.
In fact, this is the case for most JS ecosystems: express, react, nestjs...
Having been there, and also seeing others optimizing large React sites...
... it is all a waste of time. You'll get better results AND things done quicker if you swap to actually modern architecture. And with this I mean anything that will let you make the site run with as little client-side JS as possible whenever that makes sense.
My personal framework opinion leans like this:
Basically any new tool that does the zero-JS by default approach is much more likely to solve large app issues better than hyperoptimizing bundles or trying to solve the huge component tree issue. Split to islands, it actually solves the perf issues. Figure out how to share state between client-JS islands instead, if that is needed in your project. It is much easier problem to solve than making that huge React tree optimized.
What irks me the most is when I see an article that shows an example with 1 value, but you know fully well it's not gonna hold when you introduce a 2nd value...
And no matter how many times you search, you have 1 billion article for that 1 value, but none whatsoever on how to implement the 2nd or 3rd value...
An example for 1 value is the worst kind of example you can get. Always strive to have at least 2 values so we understand how to tackle multiple values in an elegant way.
I also don't write components as efficiently as I could, and then I remember why: it'll rarely make a difference except making the code more difficult to maintain.
Optimize for your humans and your manager's budget. You'd probably get more perf from simply swapping in Preact.
Well you are asking why, but you are the one who wrote this example.
I have no idea why would one store data in app state an never use it. I am not sure how this has to do with optimization if that's just a simple redundancy. You would write empty for loop, and by removing it call it optimisation?
Creating empty in-line functon wrappers instead of passing down function directly? Why did you do that?
In-line styles? Yes, components (functions) run in loops so those declarations in-line should be avoided.
When it comes to handlers declarations and memoization of the component itself. You should not memoize components that are "static". Memoization in itself isn't "free", it consumes resources. If you will wrap every static component in useMemo/useCallback, you will decrease performance of your app. What you should look into instead is how React Fiber engine works, and how to help engine recognise absolute minimum that has changed and needs a re-render. Components architecture plays role, keys, etc.
Besides you are not going to prove anything regarding performance in such generic example. What you are doing here is fixing a problem that does not exist, and by doing that with memoization you wouldn't bring any imrpvement, and likely slow things down introducing redundant complexicity (MEMO ISN'T FREE). This component is NOT slow and is not impacting performance in any way even when containing some inefficiencies you yourself introduced for unknown reasons. If you claim otherwise, I'd like to see actual metrics proving your claims, because what I see in the frontend are devs throwing empty claims regarding "performance" for their own sattisfaction while react does extremely well out of the box, and majority of carlessly written React apps are running with zero hiccups. That said useMemo/useCallback has its use, but you are not even remotely close for that usecase to be needed here.
I guess you'd much rather that I add an "example" that has several thousands lines of code.
Yeah... that'll be a great read.
Yes, but the things you have done to this component are not going to make it slower. And your supposed optimisation will show no effect.
Prove me wrong using codesanbox or something else. Feel free to write another component, prove the case exists, and let's see then how to improve it. Otherwise this is pointless - and thats my point.
I know from experience of working in alrge frontend systems, there are very rare scenarios React has difficulties to handle that majority will never stubmle upon. In general talking about "performance improvements" in frontend in a context of frameworks like React is just... I don't know. Prove the case exists, as doing unnecessery things is very much against actual engineering principals.
I honestly think once you show actual setup that may start choking 99% will realise they don't need to care about that one scenario at all.
AWESOME FEEDBACK!!!
Well written informative article..