Mojang / ore-ui

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

Cached maps (useFacetMemo) can return values before any subscription #139

Closed pirelenito closed 1 month ago

pirelenito commented 1 month ago

The choice of useFacetMap or useFacetMemo should be based on performance metrics and what is best for each use-case. So, they should share as much as possible in terms of behaviour and API.

Currently, there is one small difference between then into how they handle attempts to read their values via a get before any subscription has started.

With useFacetMap, because it has no internal caching, it always reads the source data when attempting to read its value through a get, so it would be possible to get the transformed mapped value even if no subscriptions were started.

However, with useFacetMemo because it has a cached value, it would always just return that value instead. Meaning you could get an innitial NO_VALUE with useFacetMemo when you would get the actual value with useFacetMap.

This PR fixes this inconsistency, so both hooks behave the same when trying to read their values ahead of any subscription.

pirelenito commented 1 month ago

That looks like it makes a lot of sense, we will have to double check and test to see if any users relied on the old behavior for any reason before fully merging this. but otherwise looks good 👍

This change came from us identifying a bug actually where it would only happen when using useFacetMemo.