Can We All Just Admit React Hooks Were a Bad Idea? Part 3

A Case Study

When I wrote the first two parts of this series, I predicted that in this post I would show examples from bloggers who feel confident enough that they understand hooks to provide advice to others and show how following their advice is problematic.

Last week, I spent way too many hours trying to debug an infinite loop in the React application I’ve inherited, and I decided it was better to look at how developers who were confident enough to sell their contracting services set traps for me. The code was written by multiple developers working for two different contracting companies, plus one employee who was splitting his time between the React code and several other responsibilities. I think most of these developers would be considered “good” React developers in that their knowledge seems equivalent to most developers sharing their knowledge here on Medium.

However, there was no one setting a single direction on the project, and most of them displayed no caution or skepticism around jumping in with both feet in the way that the React team suggested at any given moment. So we had a codebase that had changed direction multiple times, and because it was fragile there was code in place from every one of those directions.

I don’t know whether it was because everyone was too justifiably terrified of breaking this mess to remove the code for an old approach when a new one was started, or whether the fact that the mess frequently broke in ways that consumed way too much time in bug fixing, but the code has a mishmash of raw Context, Recoil, and Redux. Some API calls were being made in Redux thunks, a few were being made in RTK query, and the vast majority were being made inline just wherever someone decided to stick them.

In I walked in my quest to standardize on RTK Query, and 💩 hit the fan. The particular code I was editing was in a Modal. Popped up by a context menu. On a Grid Row. In a DataGrid. In another, larger component. That was in a NextJs page layout. And this is a simplified description of the component tree.

Clearly, at a minimum, something was rendering too often, but where to start? Was there a Recoil atom being updated in the background somewhere? Had someone added a key to force a rerender somewhere in the tree? Was there a hook in a hook in a cell somewhere that was causing the whole thing to rerender? Was the problem actually in the DataGrid, or could it be stemming from a hook that was in a component that was also in the layout with the DataGrid? All these were possible.

One thing I realized almost immediately was that the developers responsible for most of this did not understand that each render is entire new world, where absolutely everything is recreated on every render (and sometimes even if there is no render) unless you take specific actions to prevent that. If you don’t know what I mean, I wrote this post to get everyone on the same page:

Almost nothing was wrapped in useMemo or useCallback. I suspect they had been reading articles like this one that suggested, hey, you don’t need to do that:

The problem with articles like these is they’re only helpful to people who clearly understand the issues involved, who are unlikely to be in a position to need them. Everyone else they lead into making mistakes that someone then has to go clean up after them. Not to mention that the major reason useCallback isn’t helpful in the scenario described in the above post is that he’s still using an inline anonymous function to call the function that’s wrapped in useCallback, so it’s not accomplishing the goal of preventing extra renders by keeping the function reference stable.

He doesn’t even seem to recognize that that’s the optimization we’re trying to achieve (preventing extra renders of the child component, at the “cost” of the execution of all the statements inside the child, including creating and destroying bunches of functions). And, let’s get real, if Kent C. Dodds gets this wrong, how good do you have to be to realize you need to ignore his advice and not get it wrong yourself? Are you confident you’re that good?

Enough digression — back to my refactor. My eye was first drawn to a component that was being defined inline to be the row renderer for the DataGrid. I went to move it out-of-line more than once, but I couldn’t figure out how to give it a reference to the setState (actually the second part of a useState returned tuple, but very clumsy to talk about so let’s just call it setState and move on) that was, of course, local to the inside of the closure where the component was being defined. The React docs say not to define components inline, but then they don’t consider that you might be using a library that does a terrible job of letting you add handlers without defining the component inline.

While I considered how to handle that situation, I dutifully wrapped everything in useCallback and useMemo. It’s actually insane how much boilerplate you have to add to use something that’s supposed to be “simpler” than Class components without introducing unnecessary renders. Of course, everyone parrots the talking point that the performance implications of just letting all those rerenders happen are negligible.

Let’s just pretend you will never have a component at the top of a tree that contains a table with lots of rows and columns that makes that argument pretty lame and talk about the problem in front of us: it’s completely possible to develop problems other than performance problems caused by too many renders, including infinite loops and getting a tree that’s in a completely different state than one might suppose, because a large part of it has started over from scratch as a result of a mistake someone made somewhere. I’m not even going to mention the architectural monstrosities we get by the newly popular “pushing state down”, which is a fancy word for un-inverting dependency inversion. Hello, 1985 called and they want their anti-patterns back.

Finally, I had wrapped pretty much everything else, and I had to figure out what to do about that d⬛⬛ned inline component definition. First, I visited the docs for the Datagrid library to see if there was something the previous developers had missed. It turns out that the library designer’s intention was that you use some flavor of React context to support communication with the parent component, which is just so nasty I can’t even express how nasty I think it is🪳.

No, I’m not going to identify the library, because I think that every library would have this same problem unless they wanted to support having props for literally every kind of handler people might want to handle for the header rows and header cells and also regular rows and cells. It’s no big deal to define a component inline in a Class component, as long as you don’t do it in the render method, but the fact that each render generates an entire new definition of the component makes it a very big deal in a stateful Functional component. Which is why there are rules against it.

This is fine, I guess, until you wind up in a situation like this where no one can figure out how not to do it. Because I’m pretty sure the guys who wrote and maintained the code before I got there very well knew there were rules against it, but decided that was the lesser of the evils vs. adding yet another layer of providers to solve this problem.

I’m going to interrupt the rant already in progress to rant about how when the React team introduced the idea of hooks, one of their reasons was that it was somehow supposed to solve wrapper hell. But here we have a case where, to solve the problem in accordance with the rules that are in place because of this terrible design decision, we have to add an otherwise unnecessary Provider wrapper into the mix.

And Providers are necessary because way back when the original React team was deciding how this thing was going to work, they punted on where the business logic would go. They were pretty clear that React needed to be used in tandem with some other framework, like Redux or RxJs, to provide that functionality in an application of any complexity. But that interfered with the marketing message that React was “easy to learn,” so they introduced the single-value context as a way to strap on communication among the parts of the app without “prop drilling,” which they eventually declared war on.

So now, instead of having a common way of doing business logic in React or steering people toward business logic (called state management for marketing reasons) libraries, everyone is left cobbling together their own solution for managing a single value that’s an object through a Provider, which is a great way to infest your code with bugs, or they have to have lots of single values, each with its own Provider. And all these Providers are basically just in the way when you’re trying to write tests or use Storybook.

Or, if you don’t care about being a “cool kid”, you can do prop drilling, get the testability and ease of isolating your component in Storybook, but everyone will point and laugh.

Also, you can’t prop drill your way out of the problem I was trying to solve.

👉Eventually, I wrapped the inline Component definition in useMemo, which kept the reference stable enough to stop the endless loop, because I sure as ⬛⬛⬛⬛ wasn’t going to add another Provider there.👈

It just all felt so unnecessary, because the whole reason for the problem was these stupid stateful Functional components. And yes you could get unnecessary renders with Class components or Higher Order Components, but because their internals were stable, you could not get into an endless loop that couldn’t be diagnosed with the Profiler because the entire browser tab became unresponsive once you were in the loop.

And, yes, I have killed lots of time tracking down obscure bugs in Class components, but usually it was pretty obvious exactly which part of the code the bug was in. Not so in Functional components that are hooked into all these Contexts because people have become allergic to explicitly passing state through props. And we have people who, rather than just wrapping everything in useMemo and useCallback to be on the safe side, are now being told they need to reason about the situation to decide whether to do it or not. Which is fine, if they understand the issues in play, but even the guy most of them are reading that’s telling them appears not to understand these issues!

Problems of Using Stateful Functional Components in Enterprise Software

Leaving aside the basic design flaw of stateful functions aside, here are some other issues with hooks I run into all the time:

Nothing Has a Name

One problem hooks “solve” that nobody actually had was that logic that was supposedly related only by when it happened (specifically, in componentDidMount and componentWillUnmount) were running from the same method. But you know what? You could name those functions. You could leave a love letter to future developers that explained what those functions were for.

Now, you have a useEffect with an unnamed anonymous arrow function that, if you’re lucky, returns another anonymous nameless arrow function. But because many developers are hopelessly lost with hooks and either don’t know it or don’t want to admit it, likely that second arrow function is left off. So now that future developer gets to read every single ⬛⬛⬛⬛ing one of the 10 or 20 or more useEffects in that component and/or other components that could be the cause of the problem and try to figure out who is to blame for some problem. Big improvement 🙄.

And then good luck trying to use the dev tools to figure out what the current state of a variable in your stateful function even is. Let’s see, it’s state 1, 2, 3, 4… Probably null?

It’s Difficult to Understand What’s Going On

When you look at the small demos that are all that make sense to post in a blog post or, say, the React documentation, hooks look more straightforward than Classes or Higher Order Components. However, in a real enterprise app with lots of moving parts, it can be almost impossible to figure out why something isn’t working as expected.

Let’s start at the level of a component. Often there will be multiple useEffects, many of which set state as a result of either incoming props or other state. useEffects don’t trigger until after render, so when you have multiple ones setting state, you get render → useEffect (change state) → render → etc., over and over until finally you get to the end state the developer was aiming for. Leaving aside the performance implication of all those functions’ and other objects’ being created and destroyed multiple times, I’ve seen many cases where developers were clearly imagining that the component would jump straight to the end state and didn’t code the child components to handle all the intermediate states. Even when you do understand this is what’s going to happen, it’s super annoying to have to code around this.

Not to mention, because a state is changed after one render and then reacted to in the next render (which might change the state again), you can’t really step through this process in the debugger and see where things are going wrong. Life in the eternal “now” of eventual consistency sounds great in theory, but in practice things go wrong at a particular time, and your best shot at fixing them is to see what happened right before and right after.

This problem is magnified when functional components are used as intended in a large system, with Providers gluing everything together and hooks wrapping these hidden communication channels where they can’t be seen. It’s the height of irony to me that one of the justifications of hooks is that they’re supposed to collocate related logic so it’s right there together, and often you have to dig down multiple levels of hooks (in multiple levels of components) to find where one is triggering a change in a context, and then the reaction to it will be in a hook somewhere in a different component at some random level with no rhyme or reason.

When this happens, where do you set your breakpoints to debug a problem? How do you reason through what the original developer intended, let alone figure out how what is happening is different from that? Remember, nothing has names anymore. The only way to inform developers coming behind you (even your future self) what you intend are the much-reviled comments. And those only work when people understand what the flow really is and take the time to write them.

It’s Difficult to Figure Out Where the Fix Goes

Now, once you’ve tracked down exactly what the issue is, it’s often very unclear where you can fix it. That’s because you have components all up and down the component tree reaching out and grabbing what they need just whenever. And often they’re not doing it directly, but they’re calling into a hook. That might be combining functionality of two more hooks.

So now, you can see that the fix is somewhere in those hooks, but because the components using them know so much about them, it’s difficult to fix a problem that’s happening in one place without going into every place all those hooks are used and seeing exactly how they’re being used. Which can get exponentially difficult when hooks are used by other hooks, because you have to track down the entire tree. I can’t fathom how anyone thinks this is better than prop drilling.

Now you’ve gone through and figured out how to navigate to each place that uses the guilty hook, and we need to analyze all of that and figure out if it’s broken everywhere. If not, is there even a fix that can work and not break all of that? If there’s not, now how do we disentangle the piece of functionality that needs to work differently from that mess and put it in place?

Encourages Fragmented Logic

One of the “selling points” of hooks is that they supposedly keep related logic collocated. But that’s only true on the micro level — yes, in useEffect your subscription/unsubscription to the same notional event is collocated in a single function (sort of, we also get loads of copies of that function that are never used). But on the architectural level, the fact that hooks encourage developers to just call out for data wherever they happen to be often means there’s no overarching philosophy to how and when data is requested and parsed.

In practice, this often results in pretty random things getting passed up through callbacks (and sometimes right back down through props), and parent components holding back data they already have from children, which then fetch it again. Or weird data structures where a fetch for an entire component is happening as a result of a redux action dispatched in a child view, which then is ingested in the parent via a selector. Of course prop drilling feels scary when random, chaotic things are getting passed in and it feels like a coin flip whether you’ll still have to go get half the stuff you need anyway.

I literally just refactored such a system to use RTK Query, and the bizarre thing was no one level of the tree had all the pieces of data needed to run the new query. State was scattered all up and down 3 different levels. The only reason it worked before was because what was essentially local state for that page was all stored in Redux and all of the changes in state went through a thunk that would rerun the fetch.

Let’s go back to thinking about the design of our components first, and managing our data in a way that makes the system maintainable, not one that prevents unnecessary renders caused by hooks.

Hooks are Terrible for Testability

Ok, this is a bit of a misstatement, for brevity of the headline. What’s actually terrible for testability is the design decisions developers often have to make in response to the problems caused by the stateful component architecture (now you see why this isn’t the headline).

The two major types of design decisions either forced or encouraged by the hooks model are

• Using Context to get around the difficulty of providing stable function references from within a stateful component (as discussed in the case study) and to provide communication among the parts of Views that are organized randomly.
• Getting data too low in the component tree (aka “pushing state down”) to avoid excessive renders.

I’ll be honest with you that I believe that the testing “best practice” that’s been advocated for React is primarily because if you follow the React team’s best coding practice, traditional TDD basically isn’t possible.

Write tests. Not too many. Mostly integration.

— Kent C. Dodds

It’s a lot easier to get people to switch to a programming paradigm that’s against best practices if you can convince them that those best practices actually aren’t best practices.

But here’s the thing. Maybe you can get away with having to have an extra layer or two of Providers for a while, but what happens in the real world is someone adds a Tooltip library that needs a Provider for functionality no one really needs a test for, and a piece of Recoil state gets refactored out of the real application and no one updates the test, and then a component that wasn’t using Redux had it added because why not.

And in the mean time, no one’s paying attention to the tests. And maybe the reason they quit caring about the tests is they keep breaking. Or maybe they’re under tight deadlines and they’re not seeing productivity gains from this type of test.

Doesn’t matter why people stopped caring, when someone comes in and cares about the tests and runs them, half of them break because the implementation details that require Providers everywhere mean that your tests that weren’t maintained break. And it’s difficult to figure out exactly what needs to be fixed to update these broken tests and they all get trashed.

In the real world, it’s much easier to fix tests that break because the data that’s provided through dependency injection no longer matches how the component was implemented vs. some back-channel Provider communication is broken somewhere. And, in the real world, yes, absolutely people write tests and then someone leaves and no one cares about the tests until there’s a new hire 6 months later who cares again. I have seen broken tests no one knew about literally every place I’ve joined that already had some tests written before I got there.

And architectures need to serve real shops that work the way shops work in the real world, not just ones that follow best practices on CI/CD pipelines. Plus, those best practices are there to catch problems with implementation details, the very ones that the “mostly integration tests” philosophy is supposed to avoid. People who are passionate about testing don’t buy it, and they’re running their tests before they push their commits.

Now let’s talk about the “grabbing API data from within components that have no business doing that” problem. The argument here is that, with mostly integration tests, it doesn’t really matter where in the component tree we’re getting the data, we’re somehow saving ourselves some mocking and we can “easily” change where we’re grabbing the data from later without breaking our tests.

Well, first, you still have to get that test data into your component somehow. Whether you’re just dropping it in through props or whether you’re having to figure out how to mock out fetch or override axios, you still need to provide example data from somewhere. So where is the savings? I don’t see it.

Also, often a single piece of the view needs to have multiple scenarios tested. If you take a moment to scroll up to the graphic I swiped from “Thinking in React,” think about whether it’s easier to test a single search results item to make sure the text is red under the right circumstances by just rendering a single item and giving it different props, or whether it’s easier to wrap it in a provider that you give a series of items and it finds its own item out of the provider or whether it’s easier to have each item make a fetch call and you have to mock out a response to each call that you have to make sure gets back to the item. Or whether it’s easier to have to always create the entire View and then to test that you have to do a lot of steps that are only necessary because the sibling components need it.

And then, for each of those answers, think about what it would take to write a failing test for testing the text formatting before any code is written. For some of the above approaches, writing a failing test before any code is written wouldn’t be possible, because you have to have so much code in place before you can get to the point of writing tests.

Discourages Parallel Development

When you have a top-down data flow with property injection, it’s easier to break components out and hand them to different developers, while yet another developer works on the api calls. You can write tests that describe what the component should do in different data states, letting developers fill them in, create stub components in Storybook with the props already injected and say “make it look like the design”, or both.

By contrast, if your policy is to just reach out and grab things from wherever you happen to be, the same person has to be doing the whole thing, because no one knows up front where the developer will feel like he needs data or how that data will be massaged and parsed before it’s displayed. Likewise, with Providers it’s difficult to see how you could be working out what the Provider will do while simultaneously working on Views that reference it, unless all of that development is being done by the same person.

Tightly coupled development of the type encouraged by hooks can really impact a team’s ability to “swarm” on a feature and get it out the door quickly. This is something more and more agile-inspired methodologies insist on, so we should avoid being tied to architectures that make it difficult.

The Way Forward

A lot of people have asked me what I’d do instead. What I’m not going to recommend is that we all go back to Class-based components across the board. That ship has sailed, and there are a variety of reasons it’s unlikely to work. We should consider Classes sometimes, but we live in the world we live in, and it’s likely that most of the time, most of our code will be Functional components. Sometimes you have to build on some really rickety foundations. That’s life no matter what tools you’re using.

I have detailed ideas to make the best of this I’ll share in future posts, but here are some broad strokes of things you can do.

Take Some Time to Design Before You Code

I think the process laid out in “Thinking In React” as it relates to breaking out components before you start coding is a good one. Plan out where the data will come from and how it will flow before you start.

Keep Most Views Dumb

The original vision of React was that the ideal is for a component to get most of what it needs through props and just render that without a whole lot of fuss. I think this still makes sense. If the View is mainly concerned with displaying that and delegating actual functionality outside itself, this increases the flexibility and allows development on the Views to continue independently of the data and each other.

Just imagine if there were no such thing as a Button that took a click handler and props for a label. What if every time you wanted to have something that looks and acts like a Button, you had to start from scratch and have it figure out where to find the label and it had to go get a handler from a Provider somewhere. You wouldn’t be able to use any Button to connect to different data, and, more importantly, if you wanted to change what any given Button did, you’d have to dig down in the Button and change it there. But we think nothing of having to search around in larger component trees trying to find out where multiple streams of data are coming from.

Leave Nuance for Later

In an ideal world, everyone would understand all this stuff well enough to wield it like a scalpel, not a meat cleaver. But it’s impossible to gauge anyone’s true level of comprehension. I think I’m pretty close, but my views are completely different than most other people writing about React. Who’s right?

This means that trying to say do it this way, except in this case or that case, is not going to serve most teams very well. I say do everything in your power to prevent unnecessary renders short of adopting weird architecture that makes it hard to figure out where the data is coming from (aka push state down). This means, wrap everything in React.memo. Any object or function that’s passed into a child component should be wrapped in useCallback or useMemo, even if you need to use an inline anonymous function to make things work and it’s doing no good. No nuance!

Decide on some team guidelines about how many useStates and useEffects are too many, and stick to that number. No nuance!

If you run into performance problems from too many useMemos and useCallbacks, address it then (perhaps by injecting primitives instead of Objects, for example). But I don’t see the point of avoiding the overhead of that memoization if it causes frequent redraws of a large component tree, especially since every single object and function defined in every component definition will be created for each component instance on every render — and most of them will be garbage collected sooner rather than later, which also has a cost.

If you run out of useEffects or useStates, maybe you should switch to useMemo or useReducer or break your component down further. Or even switch to a Class-based component.

Even if you don’t like my guidelines, create some guidelines for your team that are common sense for you, not based on the end recommendations of any person or team, but on the underlying rationale for those recommendations. Because a lot of the time, recommendations don’t line up with that rationale as well as one would hope.

If you made it this far, thanks for reading! If you liked it, clap so others will also see it. You can clap more than once! Consider following me if you don’t already, or sign up to be emailed about my new posts if you do follow me. If you don’t already subscribe to Medium and you’d like an endless stream of quality content, consider using my referral link to subscribe (this helps me directly).