Fixed error "An effect function must not return anything besides a function" for useEffect

[eps1lon]

I believe it was alwas unofficially said that effect hooks (e.g. useEffect or useLayoutEffect) should either return a function or nothing i.e. undefined.

function BadEffect() {
  useEffect(function badEffect() {
    console.log("bad effect");
    return {};
  });

  return "bad effect";
}
function GoodEffect() {
  useEffect(function goodEffect() {
    console.log("good effect");
    return undefined;
  });

  return "good effect";
}

--https://codesandbox.io/s/effect-destroy-function-type-izslc?file=/src/App.js

However, React 17 and 16 did not have a runtime warning when this constraint was broken with react-dom nor did it crash when the effect was cleaned up.
React 18 will now warn when such a "bad effect" is mounted and crash when that "bad effect" should've been cleaned up.

The source code for React 17 already includes this warning but it seems like it's not actually triggered.

This post isn't necessarily about whether this warning/crash is a useful addition but rather about shining light on the new behavior/bug fix since I haven't heard anyone from the core team talk about it.

[rickhanlonii]

Yeah this was a bug fix, but I'm not sure if it was intentional that we fixed it.

The error shows in 17 if you switch to useLayoutEffect, but not for passive effects because the old effect list implementation had a special way of destroying/creating passive effects effects in a different place. This was fixed in effect list refactor, I think by @acdlite here facebook/react#20094. This change unified the path to create/destroy effects in a single place, which includes the error.

[eps1lon]

Yeah this was a bug fix, but I'm not sure if it was intentional that we fixed it.

I somewhat lean towards warning but not crashing. Seems a bit excessive to crash. But then the mistake might be missed.

The error shows in 17 if you switch to useLayoutEffect

This made me realize that the warning always mentions useEffect even if useLayoutEffect was used.

[rickhanlonii]

Oh, good point, I'll create a task.

[eps1lon]

Oh, good point, I'll create a task.

Also left a note at facebook/react#22258 (comment)