useEffect() parent-child cleanup order

[rogierschouten]

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
useEffect() runs the returned cleanup function in the wrong order between parents and child components. Say both a parent and a child component use an effect to do initialization when mounted and cleanup when unmounted. The behavior of useEffect() will be:

1. parent initialization
2. child initialization
3. parent cleanup
4. child cleanup

What is the expected behavior?

Some type of hook that allows for proper cleanup order:

1. parent initialization
2. child initialization
3. child cleanup
4. parent cleanup

[gaearon]

Can you create a demo for what you’re trying to do? It would help to know what scenario you’re trying to solve.

[rogierschouten]

My scenario is that I have a given non-react software component that can be constructed and disposed; it also has the possibility for observers to subscribe and unsubscribe themselves. This component needs to be shared between multiple React widgets.

I.e. the interface of the component is like this:

class GivenExternalComponent {
  constructor() {
  }
  
  dispose(): void {
  }

  subscribe(): void {
    if (this.disposed) { throw Error("you fool!") }
  }

  unsubscribe(): void {
    if (this.disposed) { throw Error("you fool!") }
  }
}

Now suppose I want to use this in my widgets; I have a parent widget that constructs/disposes the component and child widgets that use it:

export function ParentWidget(props: ParentWidgetProps): React.ReactElement | null {
	const givenComponent = React.useMemo(() => new GivenExternalComponent(), []);
	React.useEffect(() => {
		return () => givenComponent.dispose();
	});
	return (
          <div>
             <ChildWidget given={givenComponent}/>
             <ChildWidget given={givenComponent}/>
          </div>
       );
}

export function ChildWidget(props: ChildWidgetProps): React.ReactElement | null {
	React.useEffect(() => {
                props.given.subscribe();
		return () => props.given.unsubscribe();
	});
	return ( ... );
}

The problem with this is that the component gets disposed before the child widgets unsubscribe themselves.

Of course, any number of workarounds exist. But in general, it seems to me that the ability to properly run construct/destruct logic in reverse orders would be a good thing to have.

Actually, this is an instance of the example you have on your own site, if you were to use it recursively: https://reactjs.org/docs/hooks-effect.html

import React, { useState, useEffect } from 'react';

function FriendStatus(props) {
  const [isOnline, setIsOnline] = useState(null);

  useEffect(() => {
    function handleStatusChange(status) {
      setIsOnline(status.isOnline);
    }

    ChatAPI.subscribeToFriendStatus(props.friend.id, handleStatusChange);
    // Specify how to clean up after this effect:
    return function cleanup() {
      ChatAPI.unsubscribeFromFriendStatus(props.friend.id, handleStatusChange);
    };
  });

  if (isOnline === null) {
    return 'Loading...';
  }
  return isOnline ? 'Online' : 'Offline';
}

[illuminist]

https://codesandbox.io/s/late-sound-djog7

I'm not sure if this is a good practice as I have never used this pattern in any project before. But it's still true that the parent gets cleaned up before children can cleanup themselves when unmounting.

[rogierschouten]

Thx @illuminist and indeed that does not solve the issue yet. I really think we need an extra primitive.

[rgrzywinski]

I have nothing to add to the great write up that @rogierschouten did other than to say that this is a common case that we run into often when integrating with 3rd party APIs. We end up writing a lot of brittle code and we would love for React to support it directly.

[SteveMellross]

I think this should also apply to effects within a single component since you may have exactly the same dependencies between effects.

I'd also prefer it to be the default behavior since it seems like, if there are dependencies between effects, the cleanup would be much more likely to be stack-like than queue-like.

[ChrisChiasson]

I would like a clear statement on whether we can depend at all on the cleanup order of multiple useEffects (within the same component, and also in the child case on this issue), and if so, what that order is. (last in first out C++ wise [aka stack-like as @SteveMellross mentions], or in original declaration order)

[gaearon]

We guarantee that parent effects are destroyed before the child ones. The reason for this is that parents often tend to depend on some resource created by the child. Such as removing a listener from a DOM node managed imperatively by a child. If the child disposes its resources first, the parent might not be able to properly clean itself up. This is not specific to Hooks — it’s how componentWillUnmount works as well.

We do not make guarantees about sibling order, whether within a component or between siblings. This is because strong guarantees there hinder refactoring. Within a component, you should be able to re-order Hooks—especially, custom ones. Between siblings, it is expected that you can re-order them safely too. It is also common that only one sibling updates or unmounts, so dependencies between siblings cannot be reliable anyway.

[rogierschouten]

@gaearon this issue was not about siblings and also it did not propose to alter the useEffect hook but rather to add one with the other parent-child cleanup order too. We have use cases for it so why not add a new hook?

[gaearon]

If you'd like a new API to be added, please submit an RFC: https://github.com/reactjs/rfcs. Thanks!

[dzearing]

I think this is a real issue. Here is a sandbox to help: https://codesandbox.io/s/hook-timing-1rkz4

I observe some symmetry problems with mounting and unmounting when we compare class component lifetime vs hook useEffect and useLayoutEffect calls:

Mount:

1. useThing:useLayoutEffect mount
2. child TestComponent:componentDidMount
3. parent TestComponent:componentDidMount
4. useThing:useEffect mount <-- what?

Unmount:

1. parent TestComponent:componentWillUnmount
2. child TestComponent:componentWillUnmount
3. useThing:useEffect unmount
4. useThing:useLayoutEffect unmount

These are CLEARLY not symmetrical.

I believe the wrong one is that the useEffect call on mount should have happened as step 2.

@layershifter, @xugao

[bela53]

@gaearon Could you clarify above statement (see also SO post):

We do not make guarantees about sibling order, whether within a component or between siblings.
Within a component, you should be able to re-order Hooks—especially, custom ones.

If I understand correctly, useEffect does guarantee

• the execution order of effects in a single containing component (as written in code)
• that a child effect is executed before a parent effect
• that a parent effect is cleaned up before a child effect

useEffect does not guarantee

• the execution order of effects (incl. cleanups) of silbing components
• the execution order of effect cleanups in a single containing component (?)

Thanks!

[xialvjun]

useEffect

With code:

const C = ({n}) => {
  useEffect(() => {
    console.log('c in', n);
    return () => console.log('c out', n);
  }, [n])
  return <div>{n}</div>
}

const P = ({n}) => {
  useEffect(() => {
    console.log('p in', n);
    return () => console.log('p out', n);
  }, [n])
  return n < 7 && <C n={n} />
}

const App = () => {
  const [n, sn] = useState(0);
  return <div>
    <div><button onClick={_ => sn(n=>n+1)}>{n}</button></div>
    {n < 3 && <P n={n} />}
  </div>
}

ReactDOM.render(<App />, document.getElementById("app"));

It logs:

c in 0
p in 0
c out 0
c in 1
p out 0
p in 1
c out 1
c in 2
p out 1
p in 2
p out 2
c out 2

It's cpcc*1 + ppcc*n + pppc*1.

---

useLayoutEffect

With code:

const C = ({n}) => {
  useLayoutEffect(() => {
    console.log('c in', n);
    return () => console.log('c out', n);
  }, [n])
  return <div>{n}</div>
}

const P = ({n}) => {
  useLayoutEffect(() => {
    console.log('p in', n);
    return () => console.log('p out', n);
  }, [n])
  return n < 7 && <C n={n} />
}

const App = () => {
  const [n, sn] = useState(0);
  return <div>
    <div><button onClick={_ => sn(n=>n+1)}>{n}</button></div>
    {n < 3 && <P n={n} />}
  </div>
}

ReactDOM.render(<App />, document.getElementById("app"));

It logs:

c in 0
p in 0
c out 0
p out 0
c in 1
p in 1
c out 1
p out 1
c in 2
p in 2
p out 2
c out 2

It's cpcp*(1+n) + cppc*1.

---

Both of useEffect and useLayoutEffect should log

c in 0
p in 0
p out 0
c out 0
c in 1
p in 1
p out 1
c out 1
c in 2
p in 2
p out 2
c out 2

It's cppc*(2+n).

@gaearon

[xialvjun]

@gaearon I think the effect order matters because there are dependency relations between parent and child component.
And we don't need a new primitive.

[gaearon]

It isn’t helpful to add more comments in a closed issue. If you look closely, each previous comment asks something subtly different. So it is hard to answer consistently.

If you file a new issue with a concrete question and/or proposal, I’d be happy to discuss it. I believe this particular issue has already been answered.

[gaearon]

I’m locking — not to stifle discussion but the opposite. We have many open issues and it is impossible to track discussion in already closed ones. So this discussion simply gets lost. (For example, it’s the first time I’m seeing comments that were posted after February.)

If you want to keep discussing this, please file a new issue and keep it focused on a specific question. We’ve made some changes to the order on master too so we can discuss this as well, but the question in this issue has been answered.