Mojang / ore-ui

💎 Building blocks to construct game UIs using web tech.
https://react-facet.mojang.com/
MIT License
409 stars 18 forks source link

Move responsibility of cleaning up effects from facets to `useEffect()` / `useFacetEffect()` #96

Closed AdamRamberg closed 2 years ago

AdamRamberg commented 2 years ago

Background

I had a scenario where a useFacetEffect() (effect) depended on a facet from a useFacetMap(). I noticed that the clean up handler for the effect was sometimes called, but the effect itself wasn't called again with the new value(s). This introduced an issue to me since I was subscribing / unsubscribing to an event in the effect / clean up.

Problem

After investigating the issue I noticed that this occurs because of how the facet itself is responsible of cleaning up listeners (effects). useFacetMap() is furthermore just a thin layer on top of the facet adding a selector and an equal check. If an effect is subscribing to the useFacetMap() facet and the underlying facet updates, but the useFacetMap() is not (because of its equality check), the effect's clean up function will run, but not the effect itself with new value(s).

Solution

This PR removes the functionality of keeping track of clean up handlers in the facet itself. That logic is instead moved to useFacetEffect() / useFacetLayoutEffect(). This accomplishes 2 things:

As a bonus, this PR adds a factory function that creates the useFacetEffect() and the useFacetLayoutEffect() hooks.

Full list of changes