facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.09k stars 46.87k forks source link

Bug: Strict mode effect cleanup order doesn't match normal unmount #30765

Open xixixao opened 2 months ago

xixixao commented 2 months ago

React version: 18.2 or 18.3

Steps To Reproduce

  1. Use this code:
import { useState, useEffect } from "react";

export default function App() {
  const [show, setShow] = useState(false);
  return (
    <>
      <button onClick={() => setShow(!show)}>Toggle</button>
      {show ? (
        <A>
          <B />
        </A>
      ) : (
        "Not showing"
      )}
    </>
  );
}

function A({ children }) {
  useEffect(() => {
    console.log("parent effect");

    return () => console.log("parent effect cleanup");
  });
  return <div>{children}</div>;
}

function B() {
  useEffect(() => {
    console.log("child effect");
    return () => console.log("child effect cleanup");
  });
  return null;
}
  1. Observe the logs you get:
    // Click to show
    child effect
    parent effect
    child effect cleanup
    parent effect cleanup
    child effect
    parent effect
    // Click to hide
    parent effect cleanup
    child effect cleanup

Link to code example: https://codesandbox.io/s/sleepy-bhabha-q5tfnt?file=/src/App.js

The current behavior

Notice that during the strict mode cycle, the child effect cleanup runs before the parent's. But during actual unmounting, the child effect cleanup runs after the parent's.

The expected behavior

I expected the behavior to match.

Why does this matter

From my investigations:

  1. The expected behavior is not documented (neither for strict mode nor for normal unmount). I have searched for order and child on this page: https://react.dev/reference/react/useEffect
  2. From past issues it seems that the React team has strong opinions on what the order should be, although I'm not clear on what that order is.

The scenario we're trying to implement is for the parent to provider authentication state and for children to be subscribed and depend on this external authentication state.

Ideally we'd want:

  1. the parent's effect to run first (set authentication credentials), then children (subscribe)
  2. the child effect cleanup to run first (unsubscribe), then parent (clear authentication credentials)

We achieved the first with useLayoutEffect (isomorphic for SSR) in parent, and useEffect in child.

But now I'm realizing there might be no easy way to achieve the second, without deferring the cleanup to another tick while avoiding racing with another render.

Also note that from my testing this popular SO answer on this topic is wrong: https://stackoverflow.com/a/55028488/1526986

xixixao commented 2 months ago

Shortly after I wrote this up I had a revelation that I can control the ordering by performing my useEffect in extra child components the parent adds. So I guess the parent component does have full control over when it executes useEffects. This might be nice to document.

eps1lon commented 2 months ago

Interesting find. Do we have the same behavior in the latest React 19 RC?

gianlucadifrancesco commented 2 months ago

@xixixao I've noticed this behaviour is the same with or without StrictMode, so it doesn't seem to be related to it.

Anyway, following this @gaearon's comment, this seems expected, though the order depends on whether those components are mounted or have to be unmounted:

To demonstrate it, I've extended your reproduction and added a simple trigger to change the state:

Although I think this is 100% expected from React, maybe it should be clearer in the docs.

nkalpakis21 commented 2 months ago

built an app to get paid for this PR https://www.n0va-io.com/discover/facebook/react