thomashoneyman/purescript-halogen-hooks

Clarify `captures` documentation

milesfrain opened this issue · 1 comments

Some proposals:

  1. Add a few words about captures to this section:
    https://github.com/thomashoneyman/purescript-halogen-hooks/blob/main/docs/01-Hooks-At-A-Glance.md#the-effect-hook

  2. Change the language here to clarify that captures is not an optional performance enhancement, but that it is required. (obvious in hindsight when looking at the type signature, but this might still trip-up future readers too).
    https://pursuit.purescript.org/packages/purescript-halogen-hooks/0.4.3/docs/Halogen.Hooks#v:captures
    When I originally read "Used to improve performance", I assumed this worked like Halogen.HTML.memoized, where you can optionally choose to use it. https://pursuit.purescript.org/packages/purescript-halogen/5.0.0/docs/Halogen.HTML#v:memoized

  3. Add a note about how captures is necessary in the useTickEffect docs too. https://pursuit.purescript.org/packages/purescript-halogen-hooks/0.4.3/docs/Halogen.Hooks#v:useTickEffect
    Also, I see "array" mentioned here a lot, but the first argument looks like a record.

I agree that the captures documentation could use some improvement and with the three places you've identified as places it could see changes.

Change the language here to clarify that captures is not an optional performance enhancement, but that it is required.

This is an artifact from an older version of the code where captures produced an Array MemoValue so you could in fact provide an empty array (useTickEffect [] do ...). That's also the reason for the references to "array" rather than "record". However, in the current version you have to use captures each time, capturing an empty record if you don't have any dependencies.

I'd like to loop back around to this to update the documentation and ping you with the changes for your review.