Unmount hooks should be called during the commit phase to ensure consistency with React

[farwayer]

Describe the bug

I've run into issues several times that are caused by calling cleanup functions during the rendering phase. First time in the UI library (a similar case is described here #4222). The second time with a library that prohibits data modification at the rendering stage.

It would be nice for React compatibility and to prevent problems like this to call cleanup functions on hooks after rather than during rendering.

[JoviDeCroock]

They are called as part of the commit phase 😅 mind posting a reproduction or something we can base our discussion off of? Or do you mean deferring the hook cleanups? https://github.com/preactjs/preact/blob/main/hooks/src/index.js#L127

[farwayer]

Yes, you are right. I'm talking about calling the cleanup function of a child component while rendering the parent, if the child component is unmounted. The timing of the call is different from React and this sometimes causes problems.

let Parent = () => {
  console.log('parent render start')
  useLayoutEffect(() => console.log('parent render end'))

  let [showChild, setShowChild] = useState(true)

  useEffect(() => {
    setTimeout(() => setShowChild(false), 1000)
  }, [])

  return showChild ? <Child/> : null
}

let Child = () => {
  useEffect(() => () => console.log('child cleanup'), [])
  return null
}

React:

parent render start
parent render end
parent render start
parent render end
child cleanup

Preact:

parent render start
parent render end
parent render start
child cleanup
parent render end