matthewp / haunted

React's Hooks API implemented for web components 👻
BSD 2-Clause "Simplified" License
2.58k stars 92 forks source link

Bug when unmounted and recycling the web-component #149

Open UpperCod opened 4 years ago

UpperCod commented 4 years ago

This bug should be a comparative exercise with Atomico, in this exercise I have detected that haunted against unmounted, allows the component to continue operating even if it is not in use. This can harm the user experience since the component continues to consume resources associated with the rendering process.

In my opinion the render cycle should be paused when detecting the umounted, this opens a new problem in haunted, which is the recycling of the component, eg:

codepen

andria-dev commented 4 years ago

It looks like you're not cleaning up after doing setTimeout. If you're doing any kind of work that needs to be canceled when the element is unmounted (or when your dependencies change), you need to return a cleanup function to cancel whatever it is you're doing.

You should be able to change your effect to the following to fix your issue:

useEffect(() => {
  const id = setTimeout(() => {
    setCount(count + 1);
  }, 100);

  return () => clearTimeout(id);
});

If you're worried that could become tedious, you can always abstract out your cleanup and make a hook called useTimeout.

UpperCod commented 4 years ago

you are correct @ChrisBrownie55 , but the use of setTimeout is just an example that shows how updates are executed on the webcomponent even after having deleted it, haunted could add a renderer lock to your code when it has not been removed this would avoid loss of resources, there are asynchronous effects that cannot be correctly eliminated by useEffect without complexing your code, eg fetch, I don't use haunted, luck with this!.

andria-dev commented 4 years ago

It's actually advised to cancel your fetch request in a situation like this, and it is definitely possible but I can see how this does affect things.

For example, I was testing out my solution and when you leave the tab long enough, the Atomico example stops but the Haunted one continues

andria-dev commented 4 years ago

@UpperCod If you have any advice on how Haunted could pause the render cycle like Atomico does that would be helpful. Also what did you have in mind by "renderer lock"

UpperCod commented 4 years ago

@ChrisBrownie55 Encapsulate rerender in the constructor BaseScheduler and add a state eg, this.unmount, and execute rerender only when that state is unblocked... teardown you must change the state to generate the lock... this would solve the issue

https://github.com/matthewp/haunted/blob/master/src/scheduler.ts#L38 https://github.com/matthewp/haunted/blob/master/src/scheduler.ts#L88

As author of Atomico I'm interested in analyzing the behavior of Hooks implementations in webComponents, to achieve a conciliatory experience between libraries, I'm open to discuss the behavior to implement webcomponents+hooks