Proposal: Make the Mount component more versatile and add a component for showing and hiding

[jacobbergdahl]

Make the Mount component more versatile and add a component for showing and hiding

This is a proposal to make two primary changes to our mounting components:

1. Make the Mount component more versatile.
2. Add a performant component to show and hide JSX elements.

Note that I have not yet written any documentation for these proposed changes, but if others agree on these changes, then I will do so before this PR is merged.

Make the Mount component more versatile

Currently, the Mount component only accepts facets of type Facet<boolean | undefined>. This PR would enable the components to instead accept a more diverse number of facet types: Facet<T | null | undefined>.

The motivation for this change is to reduce code bloat while also slightly boosting performance by cutting down on redundant useFacetMap's. Note that one would still need to use a useFacetMap for more advanced conditionals.

In our current API, one often has to create a Facet<boolean> to conditionally mount elements.

const hasNoMessages = useFacetMap((numberOfMessages) => numberOfMessages === 0, [], [numberOfMessagesFacet])

return (
  <Mount when={hasNoMessages}>
    {...}
  </Mount>
)

With this updated API, this wouldn't be necessary for simple conditionals.

return (
  <Mount when={numberOfMessagesFacet} is={0}>
    {...}
  </Mount>
)

Other changes to the Mount component

This PR also introduces two other minor changes to the component.

1. Introduce a not prop.
2. Rename the condition prop to is.

Introduce a not prop

To make full use of the new versatility, one can now negate the condition.

return (
  <Mount when={myStringFacet} not={''}>
    {...}
  </Mount>
)

Rename the condition prop to is

I renamed this variable to follow the naming convention introduced by the when prop. The idea is that you write a sentence to use the component: mount when my facet is true. Arguably, the not prop could be named isNot to more closely follow this format, but I thought not sounded cleaner and still follow the rule fairly well.

Add a performant component to show and hide JSX elements

For scenarios where we really need to push the performance, it is often beneficial to show and hide elements rather than mounting and unmounting them (and without using useFacetUnwrap), and I think this should be a part of our core mounting framework to standardize the pattern. Note that there are complications to leaving elements mounted (such as memory usage and screen narration), but it is nonetheless a viable option in many scenarios.

For example, images can often be faster to show and hide than they are to mount and unmount. With our current API, this would require quite a bit of bloat.

const allyIconDisplayStyle = useFacetMap(
  (isAlly) => (isAlly ? 'unset' : 'none'),
  [],
  [isAllyFacet]
)

const enemyIconDisplayStyle = useFacetMap(
  (isAlly) => (isAlly ? 'none' : 'unset'),
  [],
  [isAllyFacet]
)

return (
  <>
    <fast-div style={{ display: allyIconDisplayStyle }}>
      <fast-img src={allyIcon} />
    </fast-div>
    <fast-div style={{ display: enemyIconDisplayStyle }}>
      <fast-img src={enemyIcon} />
    </fast-div>
  </>
)

I'm proposing a component to standardize this pattern. I am tentatively calling it Show, but as it awkwardly needs to wrap its children in an HTML element (a fast-div), I think that maybe it should be called something more explicit, such as ShowFastDiv. Would love feedback on the name and its API.

The new API would allow one to write the above code with less bloat.

return (
  <>
    <Show when={isAllyFacet}>
      <fast-img src={allyIcon} />
    </Show>
    <Show when={isAllyFacet} is={false}>
      <fast-img src={enemyIcon} />
    </Show>
  </>
)

It accepts all props that Mount would, and all props that FastDiv allows for besides style. It is possible to add support for the style prop, but I believe it would slightly reduce the performance of the component to handle it. Individual style can instead be passed down with a classname.

Remaining work

The Show component wraps its children in a div, and so it uses @react-facet/dom-fiber. I'm not sure if the Show component should really live among our other mounting components, or if it should actually live in dom-fiber, perhaps as an alternative to fast-div. In that case, it might use a different naming convention.

This package conundrum is also why the PR has failing checks.

[AdamRamberg]

I really like these changes! Great job ⭐ 👏

[pirelenito]

Closing this for now, as we have an alternative implementation that we will introduce later.