Sigh.
I’ve spent the last six hours staring at PR #8821, and I’m convinced that the “modern frontend developer” is just three script kiddies in a trench coat fueled by TikTok tutorials and a complete lack of respect for the CPU. We are running React 18.3.1 on Node 20.x, and yet, somehow, the code being submitted looks like it was written by someone who thinks “garbage collection” is a city service and not a fundamental constraint of the V8 engine.
I’m tired. My espresso is cold. My terminal is bleeding red. And you? You’re about to get a lesson in why your “clever” code is actually a pile of technical debt interest that I refuse to pay.
Here is the “God Component” that some “Senior” Engineer actually had the audacity to mark as “Ready for Review.” Look at it. Smell the rot.
// PR #8821: "Optimized User Dashboard" - My foot.
import React, { useState, useEffect, useMemo, useCallback } from 'react';
export const UserDashboard = ({ userId, theme, trackMetrics }) => {
const [user, setUser] = useState(null);
const [posts, setPosts] = useState([]);
const [config, setConfig] = useState({ mode: 'dark', notifications: true });
const [search, setSearch] = useState('');
// Problem 1: The Infinite Loop of Despair
useEffect(() => {
const fetchData = async () => {
const res = await fetch(`/api/users/${userId}`);
const data = await res.json();
setUser(data);
// This triggers a re-render, which triggers this effect again because...
trackMetrics({ type: 'view', id: userId });
};
fetchData();
}, [userId, trackMetrics]); // trackMetrics is an anonymous function passed from parent. RIP.
// Problem 2: The Stale Closure Nightmare
const handleUpdate = useCallback(() => {
console.log(`Updating user ${user?.name} with config:`, config);
// Logic that relies on 'user' but 'user' is trapped in the initial closure
}, [config]);
// Problem 3: The "I heard useMemo makes things fast" Bloat
const filteredPosts = useMemo(() => {
return posts.filter(p => {
console.log('Filtering...'); // Runs on every keystroke in the search bar
return p.title.toLowerCase().includes(search.toLowerCase());
});
}, [posts, search]);
// Problem 4: Prop-Drilling Spaghetti
return (
<div className={`dashboard-${theme}`}>
<Header user={user} />
<Sidebar config={config} setConfig={setConfig} />
<ContentArea
posts={filteredPosts}
onUpdate={handleUpdate}
search={search}
setSearch={(e) => setSearch(e.target.value)} // Anonymous function #4,002
/>
<Footer version="1.0.0" />
{/* Problem 5: The Hidden Memory Leak */}
{user && <LiveChat socketUrl={`wss://api.com/chat/${user.id}`} />}
</div>
);
};
Table of Contents
Your “Clean Code” is a Performance Tax
Let’s talk about the “God Component” above. You think you’re being “clean” by putting everything in one place. You think you’re being “modular” by using hooks. In reality, you’ve built a re-render hellscape that would make the React Fiber reconciler cry if it had tear ducts.
Most developers wouldn’t know a react best practice if it hit them in the face, and this PR is proof. You’ve ignored the fundamental way the reconciliation algorithm works. Every time trackMetrics changes—which is every single time the parent component renders because you passed it an anonymous arrow function—this useEffect fires. It fetches data. It sets state. It triggers a re-render. Which triggers the parent. Which recreates trackMetrics.
Congratulations, you’ve invented a perpetual motion machine that does nothing but heat up my MacBook and drain the user’s battery. In React 18.3.1, the Concurrent Renderer is trying its best to prioritize updates, but you’re flooding the scheduler with “urgent” work that is actually just a recursive loop of your own incompetence. You aren’t “building a feature”; you’re DOSing your own API.
Stop Using useEffect for Data Fetching, You’re Killing the Battery
If I see one more useEffect with an async function inside it that doesn’t have a cleanup function, I’m going to revoke your git push privileges. Look at that fetchData call. What happens if the userId changes while the first request is still in flight? You get a race condition. The first request finishes second, overwrites the state with stale data, and now the UI is showing User A’s dashboard while the URL says User B.
This isn’t just “buggy” code; it’s dangerous. You’re leaking data between user sessions. And don’t get me started on the memory overhead. Every time this component re-renders, you’re creating a new fetchData function, a new promise, and a new set of closures.
In a real “react best” scenario, you’d be using a cache-aware library or at the very least useSyncExternalStore if you’re dealing with a global state that exists outside the React tree. But no, you chose the “spaghetti” route. You chose to ignore the fact that useEffect is for synchronization with external systems, not for your basic CRUD logic. You’re treating the most powerful tool in the library like a window.onload event from 2004.
The Memory Leak You Call a “Feature”
Let’s look at the LiveChat component at the bottom of that mess. You’re passing a socketUrl that changes whenever user.id changes. But wait, look at the setSearch prop in ContentArea.
setSearch={(e) => setSearch(e.target.value)}
Every. Single. Render. You are allocating a new anonymous function. In a vacuum, one function doesn’t matter. But in a high-traffic library where this component might be nested inside a list of 500 items? You are choking the garbage collector. The V8 engine has to track these references, realize they’re no longer used, and sweep them. While it’s doing that, your UI janks.
You’ve created a “re-render hell” where typing a single character in the search bar causes the entire UserDashboard, the Header, the Sidebar, and the ContentArea to re-evaluate. Why? Because you didn’t memoize the props, and you didn’t split your state. Why is search (which changes every millisecond) in the same component as user (which changes once per hour)?
This is technical debt interest being compounded every time a user interacts with the page. You’re not just writing slow code; you’re writing expensive code.
Prop Drilling is a Sign of Intellectual Laziness
Sidebar config={config} setConfig={setConfig}.
Why? Why does the UserDashboard need to know how to update the sidebar’s config? This is “cruft” in its purest form. You’ve turned your top-level component into a glorified switchboard operator.
When you “prop drill” like this, you make it impossible to test components in isolation. You’ve coupled the Sidebar to the UserDashboard forever. If I want to move the Sidebar to a different page, I have to bring the config state with it, or rewrite the whole thing.
A “react best” approach would use component composition. Pass the Sidebar as a child, or use a specialized Context provider that doesn’t trigger a full-tree re-render every time a boolean flips. But that would require thinking about the component hierarchy for more than five seconds, wouldn’t it?
Concurrent Rendering Isn’t Magic Dust for Your Garbage
I see you tried to use useMemo for filteredPosts. You think you’re being “optimized.” You’re not. You’re memoizing a filter operation on an array that probably has ten items in it. The overhead of the useMemo hook—the dependency array comparison, the memory allocation for the memoized value—is likely more expensive than just running the .filter() call.
And then there’s useDeferredValue. I know you didn’t use it here, but I know you’re tempted to throw it at the search state to “fix” the lag. Let me be clear: useDeferredValue is not a “make it fast” button for your bloated components. It’s a tool for the Concurrent Renderer to keep the input field responsive while the heavy lifting happens in the background. If your “heavy lifting” is just a poorly written map function, useDeferredValue is just putting a band-aid on a gunshot wound.
You need to understand the “work loop.” React 18 processes updates in “lanes.” When you trigger a state update, React tries to find a slice of time to do that work. If you provide a “spaghetti” of dependencies, you’re forcing React to abandon its concurrent optimizations and fall back to synchronous, blocking renders. You are literally fighting the library you’re trying to use.
Your State Management is a Rube Goldberg Machine
const [config, setConfig] = useState({ mode: 'dark', notifications: true });
Why is this an object? Every time you want to change the mode, you have to spread the previous state: setConfig(prev => ({ ...prev, mode: 'light' })). If you forget the spread, you lose the notifications key. This is a classic source of bugs, and for what? To save one line of useState?
This is what I call “bloat.” You’re adding complexity where none is needed. Derived state is another one. I bet if I looked at the Header component, I’d find another useEffect that calculates the user’s “display name” based on the user prop.
Newsflash: If you can calculate it during render, calculate it during render. Don’t put it in state. Don’t put it in an effect. Just. Calculate. It. The CPU is good at math; it’s bad at keeping track of your redundant, out-of-sync state variables.
Terminal Output: The Reality Check
Here is what happened when I tried to run your PR against our CI/CD pipeline. It didn’t even make it past the build stage.
$ npm run build --project dashboard-v2
> [email protected] build
> tsc && vite build
src/components/UserDashboard.jsx:22:11 - error TS2345: Argument of type '() => Promise<void>' is not assignable to parameter of type 'Destructor'.
22 | useEffect(() => {
| ^
23 | const fetchData = async () => { ... }
24 | }, [userId, trackMetrics]);
[ERROR] Memory limit exceeded: V8 heap limit reached during minification.
Heap usage: 4096MB / 4096MB
[STDOUT] FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
1: 0x1012e3450 node::Abort() (.?AVAbort@node@@)
2: 0x1012e4560 node::OnFatalError(char const*, char const*) (.?AVOnFatalError@node@@)
3: 0x101456780 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool)
4: 0x101456a10 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool)
5: 0x101600000 v8::internal::Heap::AllocateRawWithRetryOrFailSlowPath(int, v8::internal::AllocationType, v8::internal::AllocationOrigin, v8::internal::AllocationAlignment)
[STACK TRACE]
react-dom.development.js:24560 (reconcileChildren)
react-dom.development.js:18230 (beginWork)
react-dom.development.js:22340 (performUnitOfWork)
react-dom.development.js:22310 (workLoopSync)
-- Memory Leak Detected in <LiveChat /> --
Leaked Objects: 4,502 (EventEmitter listeners not removed)
Look at that. 4,502 leaked listeners. You didn’t clean up your WebSocket connection in the LiveChat component. Every time the userId changed, you opened a new connection without closing the old one. You’re not just crashing the browser; you’re probably crashing our staging socket server too.
The Manifesto of Minimalist React
I am tired of being the “grumpy” one, but if this is the quality of code being produced by “modern” developers, then someone has to hold the line. Here are my rules. If your PR violates these, I will close it without comment.
- No Anonymous Functions in Props. If I see
onClick={() => ...}in a component that renders more than once, I’m rejecting it. UseuseCallbackor move the function outside the component. - useEffect is a Last Resort. If you can use
useMemo, use it. If you can calculate it during render, do it. If you’re fetching data, use a real data-fetching library that handles caching and invalidation. - Primitive State Only. Stop wrapping everything in objects.
useState(true)is better thanuseState({ active: true }). It’s easier to track, easier to update, and less prone to “stale closure” bugs. - Cleanup is Mandatory. If you open a socket, close it. If you set a timeout, clear it. If you add an event listener, remove it. This isn’t “react best” practice; it’s basic programming.
- Boring is Better. I don’t want to see your custom hook that abstracts away the
fetchAPI with 15 layers of generics. I want to see code that a junior can read and a senior can’t break. - Respect the Fiber. Understand that React 18 is a scheduler. Don’t block the main thread with heavy computations. If you have to do something slow, use
useTransitionoruseDeferredValuecorrectly, or better yet, move it to a Web Worker.
The “Boring” Fixed Version
Here is how you write that component without being a menace to society. It’s boring. It’s standard. It actually works.
import React, { useState, useEffect, useMemo, useCallback } from 'react';
import { useUserData } from './hooks/useUserData'; // Abstracted fetch with cleanup
import { usePosts } from './hooks/usePosts';
// Move static config outside to prevent re-allocation
const DEFAULT_CONFIG = { mode: 'dark', notifications: true };
export const UserDashboard = ({ userId, theme }) => {
// Split state: search changes often, config doesn't
const [search, setSearch] = useState('');
const [config, setConfig] = useState(DEFAULT_CONFIG);
// Custom hook handles the race conditions and cleanup
const { user, isLoading } = useUserData(userId);
const { posts } = usePosts(userId);
// Memoize only when the calculation is actually expensive
const filteredPosts = useMemo(() => {
const query = search.toLowerCase();
if (!query) return posts;
return posts.filter(p => p.title.toLowerCase().includes(query));
}, [posts, search]);
// Stable reference for event handlers
const handleSearchChange = useCallback((e) => {
setSearch(e.target.value);
}, []);
const handleUpdate = useCallback(() => {
if (!user) return;
console.log(`Updating user ${user.name}`);
}, [user]);
if (isLoading) return <Spinner />;
return (
<div className={`dashboard-${theme}`}>
<Header user={user} />
<Sidebar config={config} onConfigChange={setConfig} />
<ContentArea
posts={filteredPosts}
onUpdate={handleUpdate}
search={search}
onSearchChange={handleSearchChange}
/>
<Footer />
{user && <LiveChat userId={user.id} />}
</div>
);
};
// LiveChat must have a cleanup!
const LiveChat = ({ userId }) => {
useEffect(() => {
const socket = new WebSocket(`wss://api.com/chat/${userId}`);
return () => socket.close(); // CLEAN YOUR MESS
}, [userId]);
return <div className="chat-window" />;
};
Notice the difference? No infinite loops. No memory leaks. No prop-drilling of setters. The state is split so that typing in the search bar doesn’t re-trigger the user data logic. The WebSocket actually closes when the component unmounts.
It’s not “clever.” It’s not “vibrant.” It’s just correct.
Now, take your original PR, delete the branch, and go read the React documentation on “Synchronizing with Effects” until your eyes bleed. Don’t come back until you’ve learned that “Clean Code” isn’t about how many hooks you can cram into one file—it’s about how little code you can write while still solving the problem.
I’m going to get more espresso. Don’t touch anything.
Related Articles
Explore more insights and best practices: