thomashoneyman/purescript-halogen-hooks

For readability in custom hooks, export type signatures of various functions via type aliases

JordanMartinez opened this issue · 4 comments

When writing custom hooks, the type signatures can get really ugly. I think it would be useful to define type aliases to the type signatures of some functions, such as useTickEffect and capturesWith.

For example, without such a thing, this is what the full type signature of subscribeTo' looks like:

subscribeTo'
  :: forall a m output slots
   . { push :: a -> HookM slots output m Unit
     , props :: { capturesWith :: CapturesWithEqFn rows
                               -> (MemoValues -> HookM slots output m (Maybe (HookM slots output m Unit))
                               -> (forall hooks. Hooked slots output m hooks (UseEffect hooks) Unit))
                               -> HookM slots output m (Maybe (HookM slots output m Unit))
                               -> (forall hooks. Hooked slots output m hooks (UseEffect hooks) Unit)
                , subscribe :: (a -> HookM slots output m Unit) -> HookM slots output m Unit
                }
     }
  -> ({ state :: Maybe a } -> { state :: Maybe a } -> Boolean)
  -> (a -> HookM slots output m Unit)
  -> Hook slots output m UseEffect Unit

Here's what it looks like when we add type aliases:

subscribeTo'
  :: forall a m output slots
   . { push :: a -> HookM slots output m Unit
     , props :: { capturesWith :: CapturesWith slots output m (state :: Maybe a)
                , subscribe :: (a -> HookM slots output m Unit) -> HookM slots output m Unit
                }
     }
  -> CapturesWithEqFn (state :: Maybe a)
  -> (a -> HookM slots output m Unit)
  -> Hook slots output m UseEffect Unit

-- | Type signature for the `eqFn` value in
-- | ```
-- | Hooks.capturesWith eqFn { state } Hooks.useTickEffect do
-- |   someHookMAction
-- | ```
type CapturesWithEqFn rows = { | rows } -> { | rows } -> Boolean

-- | Type signatures for `Hooks.capturesWith`.
type CapturesWith slots output m rows =
  CapturesWithEqFn rows
    -> (MemoValues -> UseTickEffect slots output m)
    -> UseTickEffect slots output m

I agree this reduces the verbosity of something like subscribeTo', but I don't think that I would like to add this to the Hooks library for a few reasons:

  1. I think the Fairbairn threshold applies to type signatures as well, especially type synonyms; I try to only add type synonyms when it reduces verbosity for a very common case, but not if it makes things nicer for a special use case. I don't yet think that libraries exporting functions using the captures function is a very common case. But I could change my mind in the future!

  2. Users can add this type synonym to their code if they find it improves the readability of the function they're writing, like subscribeTo'. It's not needed that the Hooks library itself implements this, and as I've maintained more libraries over time I've found it's a good idea to keep the library core small when the cost is not high. Essentially, this puts (1) in the hands of the user writing this case; users in app code won't do this very often, if ever.

  3. As a matter of style, I don't like type synonyms that hide functions except for very commonly-established patterns or newtypes needed for instances like ReaderT. I find that it makes it harder to see what arguments you need to supply and where and I end up having to look up the functions within the type. This might just be my own style, but for Hooks I'd like to stick to it where possible.

I'm willing to disregard these points if the benefit seems to be there, namely that introducing the new types will significantly ease a common situation (esp. for app users, not library authors) and it's not easy to implement the same thing in the library / app.

For example, if it turns out every Hooks-based library ends up doing something like this, then I'd be a lot more open to introducing this to the library itself. But it's too early to say that now.

I agree. It's too early to say.

I'll close this for now. If we ever want to reconsider it again after more time has passed, let's reopen and continue discussion. I'll also update halogen-hooks-extra to remove that type signature.

At the very least, can we fix the type signature for captures and capturesWith? While they are accurate as a can resolve to HookM slots output m (Maybe (HookM slots output m Unit)) -> (Hook slots output m UseEffect Unit), can we be more explicit with that type signature? That is partly what led me astray when working on migrating select to a hook version.