DEV Community

loading...
Cover image for Clean Up Your useEffect, But Not Too Much

Clean Up Your useEffect, But Not Too Much

derekmt12 profile image Derek N. Davis Originally published at derekndavis.com ・2 min read

Like a lot of developers, I love to write code that is to the point and stripped of all the fluff. Something about getting code down to its smallest form is really satisfying. Although, at a certain point, conciseness and maintainability are tugging on opposite ends of the same strand.

Where this has particularly bitten me is with the cleanup function of a useEffect.

The Scenario

We start with a really simple useEffect.

useEffect(() => thing.register(), []);
Enter fullscreen mode Exit fullscreen mode

Nothing special, right? Well, let's say later on we come back in here and decide brackets would look nicer, so it gets changed.

useEffect(() => {
  thing.register();
}, []);
Enter fullscreen mode Exit fullscreen mode

Except... we have a problem now. These do not behave the same way. What we forgot is that thing.register() actually returns an unregister function that needs to be called in the effect cleanup. So what we should have done was this:

useEffect(() => {
  // now works the same as the implied return
  return thing.register();
}, []);
Enter fullscreen mode Exit fullscreen mode

Conciseness vs. Maintainability

Let's consider this setup though.

Will we (or anyone else on our team) remember in 6 months that register() returns an unregister function that useEffect will call in its cleanup? We can only hope. The implied return in that first example makes it even more "magic."

Instead of hoping we remember that, let's instead create an intermediate variable to make it more clear.

useEffect(() => {
  const unregister = thing.register();
  return unregister;
}, []);
Enter fullscreen mode Exit fullscreen mode

It's not as concise as the original, but I could come back after a long period of time and know exactly what that code is doing.

Summary

  • After refactoring, consider the impact to the maintainability of your code.
  • Make it clear when a useEffect has a cleanup function to avoid future defects.

Discussion (1)

Collapse
lukeshiru profile image
LUKE知る

You could also just...

useEffect(thing.register, []);
Enter fullscreen mode Exit fullscreen mode

And just to be safe if that thing value changes...

useEffect(thing.register, [thing]);
Enter fullscreen mode Exit fullscreen mode

I get the point of having an unregistet const to make clear that you're returning that, but the shortest approach is also quite simple to read and understand: "thing.register is a side effect".

Forem Open with the Forem app