DEV Community

Cover image for The clean coding design mistake of `useEffect`
András Tóth
András Tóth

Posted on

The clean coding design mistake of `useEffect`

I saw a lot of arguments over object-oriented principles (namely S.O.L.I.D. ones), MVC and MVVM based critiques of the hooks of React but none from the most basic clean coding ones.

Your function should do one thing and that should be obvious.

Disclaimer: I do not wish to bash React hooks, I use them and they are quite effective. I do wish to point out that instead of writing smart code that can do hypothetically anything let's spend brain power on writing obvious code.

The principle of the least astonishment you know...

The 3 wildly different things you can do with useEffect

Run some side effect at every render

I think this one is pretty clean!

function Stuff() {
  useEffect(() => document.title = 'I run on every render!');

  return <div>stuff!</div>
}
Enter fullscreen mode Exit fullscreen mode

Run some side effect when a dependency changes

This is where things get wild for every newcomer to React! The useEffect thingy can not always run!

function Stuff({ randomId }: properties) {
  useEffect(() => {
   // this should update something as well on the UI
   // but I omit many moving parts for brevity
   fetchThatThing(randomId);
  }, [randomId]);

  return <div>stuff!</div>
};
Enter fullscreen mode Exit fullscreen mode

To whom this is familiar, this is straightforward:

  • you run a side-effect when randomId is changing
  • you make sure it only runs by specifying said randomId in an array

But then people who have just read the docs a bit and did not scroll down to the relevant parts are going to do one of these:

function Stuff1({ randomId }: properties) {
  useEffect(() => {
   fetchThatThing(randomId);
   // running fetchThatThing at every render!
  });

  return <div>stuff!</div>
};

function Stuff2({ randomId }: properties) {
  useEffect(() => {
   fetchThatThing(randomId);
   // passing the dependency the wrong way!
  }, randomId); // or { randomId } for total chaos

  return <div>stuff!</div>
};
Enter fullscreen mode Exit fullscreen mode

We can choose in this case to express our superiority as we have spent hard times to read all the docs and do all the trainings, or just silently correct our fellow's mistake.

Side note: do not expect React proficiency!

You can argue that it is easy to remember these 3 gotchas.
But let me give you another viewpoint!

Let's say you work in a large corporation with multitudes of developers, weak ownership of code i.e. anyone can create a pull request to any repository.

You will have teams working with Vue.js, AngularJS or whatever. And sometimes these folks would need to make a little change in your React application.

They can be even seasoned frontend developers used to know other frameworks quite intimately, but not React in particular. Or they use learned React, but since they are full-stack they just touch the framework every 3 month.

In a real-world setup you cannot expect everybody to speak fluent React, you have to expect a highly varied depth of knowledge. Any design that expects developers to be intimately familiar with gimmicks of a framework will create friction: declined pull-requests, reworks, time wasted on ironing gotchas.

And that's precisely why you have to make everything obvious, so people are not going to make super avoidable mistakes.

Run a side effect only at first render

There are initialization steps in almost every application and then people are going to ask you the React guru of the company:

"Hey, I just need to run this script once at the start of the app, but that damn useEffect keeps running it all the time!"

You roll your eyes and tell that they just need to pass an empty array [] as the dependency. "How obvious!" Said nobody ever.

function Stuff() {
  useEffect(() => {
   // so now this thing will run once the 
   // first time it is rendered 
   fetchThatThing(randomId);
  }, []);

  return <div>stuff!</div>
};
Enter fullscreen mode Exit fullscreen mode

A little story before I go for the solution

I started my real development career as a software automation engineer. We needed to write scripts using a UI automation framework that was clicking buttons and waiting until navigation happened.

One of the automation teams started innovating and realizing they were really clicking and waiting for a button to disappear very frequently. To them it occurred they can just merge these two in function call:

click(waitForDisappearing: boolean) { ... }

I am omitting details, but this is how it looked when you used it:

// let's click on "Register" button
// and then wait until it is unloaded from the screen
registerButton.click(true);
Enter fullscreen mode Exit fullscreen mode

Well, what??? What is a click(true)? A true click? Are there false, evil, deceptive clicks as well?

They violated a very important principle:

Do not pass flags and special parameters that is wildly changing the behaviour of the function in non-trivial ways! If you want to use flags, create separate functions.

So they could have just done this:

// this can be read and understood by your manager...
registerButton.clickAndWaitUntilItDisappears();
Enter fullscreen mode Exit fullscreen mode

Simple and obvious.

What they should have done with useEffect

This is where I say: remembering less is harder than remembering a lot.

Less functions are sometimes harder than more functions to remember.

I don't see what problem it would have caused to do this:

function Stuff({ randomId }: props) {
  useEffectOnInit(() => {
   // no need to pass in `[]`
   // runs at first render
  });

  useEffectPerRender(() => {});

  useEffectOnPropChange(
    () => {}, 
    // Mandatory array! 
    // Run time warning if empty!
    [randomId]
  );

  return <div>stuff!</div>
};

Enter fullscreen mode Exit fullscreen mode

The naming is a bit clunky, but uses terminology every developer is familiar with. Obviously with the given time and alternatives it can be nailed better.

But you get the point.

When those devs from that other team come and copy-paste code from around your project to fulfil their project requirements, they will have no problem understanding what any of these mean and how to interact with them.

Wrap up

Remember: clean coding is not an object oriented programming paradigm. It is a set of hard earned programming user experience tips that every developer should know and use in their coding.

By shortening the length of functions required to learn, they created a very steep learning curve for folks who are new or rarely use React.

If you still profoundly disagree with me, grab a backend javascript developer by the arm put them down on a chair and explain them these 3 use cases of useEffect. Don't forget to watch their faces! I saw their reaction many times 😅. "Frontend is stupid/too crazy to learn!"

Latest comments (0)