Skip to content

Add useSteppingAff hook #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 30, 2023
Merged

Add useSteppingAff hook #72

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions src/React/Basic/Hooks/Aff.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module React.Basic.Hooks.Aff
( useAff
, useSteppingAff
, UseAff(..)
, useAffReducer
, AffReducer
Expand Down Expand Up @@ -37,11 +38,33 @@ useAff ::
deps ->
Aff a ->
Hook (UseAff deps a) (Maybe a)
useAff deps aff =
useAff = useAff' (const Nothing)

--| A variant of `useAff` where the asynchronous effect's result is preserved up
--| until the next run of the asynchronous effect _completes_.
--|
--| Contrast this with `useAff`, where the asynchronous effect's result is
--| preserved only up until the next run of the asynchronous effect _starts_.
useSteppingAff ::
forall deps a.
Eq deps =>
deps ->
Aff a ->
Hook (UseAff deps a) (Maybe a)
useSteppingAff = useAff' identity

useAff' ::
forall deps a.
Eq deps =>
(Maybe (Either Error a) -> Maybe (Either Error a)) ->
deps ->
Aff a ->
Hook (UseAff deps a) (Maybe a)
useAff' initUpdater deps aff =
Copy link
Member

@pete-murphy pete-murphy May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about exporting useAff' to allow a user to define custom hooks built on top of it? Contrived example, but I could imagine maybe wanting to pass something other than identity or const Nothing as the initUpdater if wanting to track loading state along with the Aff's result (to preserve and render the previous result but disable a button while some request is in flight, e.g.)

useReloadingAff
  :: forall deps a
   . Eq deps
  => deps
  -> Aff a
  -> Hook
       (UseAff deps { isLoading :: Boolean, result :: a })
       (Maybe { isLoading :: Boolean, result :: a })
useReloadingAff deps aff =
  React.useAff' initUpdater deps aff'
  where
  initUpdater = (map <<< map) (_ { isLoading = true })
  aff' = map { isLoading: false, result: _ } aff

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd want to have a pretty convincing use case. It's easy enough to copy/paste for current edge cases, so I worry it's not worth exposing that complexity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But happy to defer to others since I'm not an active user right now.

Same goes for merging/publishing this.

Copy link
Member

@pete-murphy pete-murphy May 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easy enough to copy/paste for current edge cases, so I worry it's not worth exposing that complexity.

Keeping the external API simple makes sense to me 👍 In cases where I've wanted anything other than the Maybe a state that useAff returns I'd just use a separate useState and use useAff for scheduling. So for the snippet I had above I think I'd probably just do

useReloadingAff deps aff = React.do
  state /\ setState <- useState Nothing
  useAff deps do
    liftEffect $ setState (map (_ { isLoading = true }))
    result <- aff
    liftEffect $ setState \_ -> Just { isLoading: false, result }
  pure state

which is not much more complicated than the version using useAff'.

I'm not really an active user either aside from side projects. Lumi's use case for useSteppingAff seems like enough motivation for merging this as-is, but I'll leave this open for feedback for a couple more days before merging.

coerceHook React.do
result /\ setResult <- useState Nothing
useEffect deps do
setResult (const Nothing)
setResult initUpdater
fiber <-
launchAff do
r <- try aff
Expand Down