-
Notifications
You must be signed in to change notification settings - Fork 47.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug: the order of effect and cleanup in Parent and Child component is weird #19482
Comments
Although react lifecycle order works right, it lacks Related issue: #6424 . This issue is closed because
But HookApi doesn't solve the problem of parent component can not clean up resource shared by its children. |
To solve the problem of parent component can not clean up resource shared by its children, there is a hack way: #6424 (comment) And by imitating the above method I wrote a hook which can express 6 lifecycles: 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 |
What about using |
If useing |
True. It doesn't solve the problem well, since the |
Any news on this? I'm also using Mapbox and wanted to do the same thing and hit the same problem. |
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. |
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! |
bump |
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! |
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! |
React version: v16.12.0
Steps To Reproduce
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:Then I want to make a react version:
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:
#16728
#17080
And the react lifecycle order is works right: https://stackblitz.com/edit/react-lifecycle-effect-order-right?file=index.js
The text was updated successfully, but these errors were encountered: