Bug: the order of effect and cleanup in Parent and Child component is weird

[xialvjun]

React version: v16.12.0

Steps To Reproduce

1. just as the code example shows: just click the button 6 times respectively and see the console

Link to code example: https://stackblitz.com/edit/react-effect-order-matters?file=index.js

The current behavior

useEffect logs: cpcc*1 + ppcc*n + pppc*1 useLayoutEffect logs: cpcp*(1+n) + cppc*1

The expected behavior

both useEffect and useLayoutEffect should log: cppc*(1+n+1)

Why I need a stable effect and cleanup order

There is a vanilla js package mapbox-gl whose use case is:

import mapboxgl from 'mapbox-gl';
var container = document.querySelector('#map');
var map = new mapboxgl.Map({ container });

map.on('load', function () {
  // every thing should be done after load
  map.addSource('route_source', route_source_data);
  map.addLayer({
    source: 'route_source',  // the layer relies on the source
    id: 'route_layer',
    ...other_layer_config,
  });
  // ...;
  do_many_things();
  // ...;
  // you must remove the layer before removing the source
  map.removeLayer('route_layer');
  map.removeSource('route_source');
});

Then I want to make a react version:

var vdom = (
  <Map opts={x}>
    <Source opts={xx}>
      <Layer opts={xxx}></Layer>
    </Source>
  </Map>
)

But since the order of react effect and cleanup in parent and child component is not stable (well, it' stable but it's weird), things became complex.

I think the effect and cleanup order matters because there are dependency relations between parent and child components.

And react should handle it.

There are some related(maybe) issues: https://github.com/facebook/react/issues/16728 https://github.com/facebook/react/issues/17080

And the react lifecycle order is works right: https://stackblitz.com/edit/react-lifecycle-effect-order-right?file=index.js

[xialvjun]

@gaearon just like this comment, I filed a new issue. And in this issue, I gave a real use case, hoping this will draw you guys' attention.

[xialvjun]

Although react lifecycle order works right, it lacks componentDidUnmounted.

Related issue: https://github.com/facebook/react/issues/6424 . This issue is closed because

Closing as React's design is moving away from class-based lifecycle methods

But HookApi doesn't solve the problem of parent component can not clean up resource shared by its children.

[xialvjun]

To solve the problem of parent component can not clean up resource shared by its children, there is a hack way: https://github.com/facebook/react/issues/6424#issuecomment-351760732

And by imitating the above method I wrote a hook which can express 6 lifecycles: willMount, didMount, willUpdate, didUpdate, willUnmount, didUnmount. And the parent children lifecycles running order is right: Parent willMount, Child willMount, Child didMount, Parent didMount, Parent willUpdate, Child willUpdate, Child didUpdate, Parent didUpdate, Parent willUnmount, Child willUnmount, Child didUnmount, Parent didUnmount, it's pccp*(1+n+1) which is cppc*(1+n+1)(this issue's useEffect expected behavior) transform to pc + cppc*(1+n+1) + cp

type life = "mount" | "update" | "unmount";
export const useWillDid = <T extends (life: life) => void | ((life: life) => any), D extends any[]>(
  fn: T,
  deps?: D,
) => {
  const inv = useRef<void | ((life: life) => any)>();
  const mounted = useRef(false);
  const is_last = useRef(false);
  is_last.current = false;
  inv.current = useMemo(() => {
    return fn(mounted.current ? "update" : "mount");
  }, deps);
  useEffect(() => {
    if (inv.current) {
      inv.current(mounted.current ? "update" : "mount");
    }
    mounted.current = true;
    is_last.current = true;
    return () => {
      if (is_last.current) {
        inv.current = fn("unmount");
      }
    };
  }, [inv.current]);
  const style = useMemo(() => ({ display: "none" }), []);
  const ref = useMemo(
    () => (ele: any) => {
      if (!ele && inv.current) {
        inv.current("unmount");
      }
    },
    [],
  );
  return <span ref={ref} style={style}></span>;
};

Even though I have wrote this hook which can solve my problem, I still thought It should be done by react. Hope you consider it more.

[mbest]

What about using setTimeout in the parent's cleanup function?

[xialvjun]

If useing setTimeout, what about parent's parent?

[mbest]

If useing setTimeout, what about parent's parent?

True. It doesn't solve the problem well, since the setTimeout callbacks will run in the wrong order.

[derzwen]

Any news on this? I'm also using Mapbox and wanted to do the same thing and hit the same problem.

[tengl]

We also have this problem. I've made a simple React app with maplibre to demonstrate the issue, and two possible work arounds. If someone has a better solution I'd be happy to hear it.

[github-actions[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[xialvjun]

bump

[github-actions[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[github-actions[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!