Clarify `captures` documentation
milesfrain opened this issue · 1 comments
Some proposals:
-
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 -
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 likeHalogen.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 -
Add a note about how
captures
is necessary in theuseTickEffect
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.