Mojang/ore-ui

RFC: Change strategy on "default equality checks"

Closed this issue · 0 comments

There is room to improve a bit further our strategy regarding equality checks. Right now we have a "default equality check" that is applied everywhere, but this default implementation adds a runtime type check on every useFacetMap and useFacetMemo we add.

The goal here is to remove the overhead and align the behavior of equality checks to what is expected from "vanilla React". React has no memoization by default, and we can probably lean on the same premise.

useFacetMap and useFacetMemo

Change to have NO equality checks set by default, and if needed it should be added explicitly by the user. This will make our map implementation much closer to React's component (React re-renders a component if called with the same values).

useFacetState

Here, we have two options:

  • We could go "immutable" and behave exactly as React by default. Meaning that if you set the state with the same reference, it doesn't trigger an update. Given this is a local state, it might be safe to take this approach, mutability is only an issue with "remote facets".
  • Another option would be to remove completely the equality checks, and any "set" would trigger an update (even with the same reference). This would diverge from React, but it might be more friendly to use in conjunction with "remote facets".

With either approach we would still support being explicit about which equality check to use (as it is today).

useFacetEffect

The proposal would be to maintain the behavior we have today, but as an "effect" of the previous changes it would mean for users our useFacetEffect would behave differently from useEffect.

Given the equality checks would be removed from the maps, effects would always be triggered, even for updates with the same reference.

We could extend the API to support equality checks, but given we can have multiple facets as a dependency, it would become quite verbose to setup any checks.

So, to prevent effects being fired, users would need to add equality checks on outer layers (ex: useFacetMap), if needed.

dom-fiber and dom-components

To support this change, our custom renderer would need to start performing equality checks on each prop to know for sure if the value of a facet have changed. The benefit here is that we would know for sure the type of the Facet and can do strict equality checks.