purescript-react / purescript-lumi-components

Library of our UI components
https://lumihq.github.io/purescript-lumi-components/#/
Apache License 2.0
105 stars 5 forks source link

Form listen is not observably asynchronous #229

Open jship opened 11 months ago

jship commented 11 months ago

The listen function in the form code is not observably asynchronous from a UI perspective: https://github.com/lumihq/purescript-lumi-components/blob/1ebe3831bf44130fe007aab8c1e1fdb85fbb578b/src/Lumi/Components/Form/Internal.purs#L257-L268

The recent upgrade of PS and React included updates to this function's docs to indicate this.

Lumi built some internal variants of listen that may be worth upstreaming here. listenWith is observably asynchronous, though has a caveat in regards to its update behavior described in detail in the docs:

type Settings :: Type -> Type
type Settings a =
  { debouncer :: Debouncer (a -> a)
  , buildCallback :: a -> a -> Maybe (Aff (a -> a))
  }

-- | Listens for changes in a form's state and allows for performing debounced
-- | asynchronous effects and additional state changes.
-- |
-- | The asynchronous effect is fired on every state change. If a previous
-- | instance of the effect is already queued and the state changes, the
-- | previous instance of the effect is canceled and replaced with the latest
-- | instance.
-- |
-- | Note that this function requires care to be used safely. To keep the UI
-- | responsive to the user, the wrapped form's UI will be updated immediately
-- | regardless of whether or not the callback builder returns a callback. This
-- | keeps the input field the user is interacting with from feeling laggy. If
-- | the callback builder returns a callback, it will be scheduled for
-- | (debounced) asynchronous execution and when execution completes, the
-- | wrapped form's UI will be updated a second time. The callback builder is
-- | given the previous state and the current state, which enables computing a
-- | diff to determine whether or not an asynchronous response to the state
-- | change is warranted.
-- |
-- | It follows that the user-specified callback builder must only return an
-- | action when it detects a state change in which it would be safe for the
-- | form UI to be updated twice. Concretely, if the callback builder ignored
-- | the specific state change and always blindly returned an action, a change
-- | like adding a row to a table would be duplicated: one row would be added
-- | immediately, and a second row would be added after the asynchronous action
-- | completes.
-- |
-- | An example of a state change where it is generally safe for the form UI to
-- | be updated both immediately and after the asynchronous effect has executed
-- | is when the user is typing into an input field. Both UI-updating effects
-- | observe largely the same core state change that initiated the listener in
-- | the first place: the text in the input. While it is technically possible
-- | for the asynchronous action's returned updater function to change the state
-- | such that the second UI-updating effect does not observe the same text that
-- | was observed by the first UI-updating effect, this would be a useless
-- | listener as the user's input would be changed out from under them as they
-- | were typing. Considering it is exceedingly unlikely for a listener to be
-- | implemented in that way, we can reasonably assume both UI-updating effects
-- | observe roughly the same core state change, and the second UI-updating
-- | effect observes whatever additional changes were made by the asynchronous
-- | action's returned updater. Caveat emptor!
listenWith
  :: forall ui props unvalidated result
   . Settings unvalidated
  -> FormBuilder' ui props unvalidated result
  -> FormBuilder' ui props unvalidated result
listenWith { debouncer, buildCallback} (FormBuilder form) =
  FormBuilder \props state ->
    let { edit, validate } = form props state
     in { edit: \(onChange :: (unvalidated -> unvalidated) -> Effect Unit) ->
            edit \(update :: unvalidated -> unvalidated) -> do
              for_ (buildCallback state $ update state) \getUpdate -> do
                Debouncer.run debouncer
                  { action: do
                      updateFromCallback <- getUpdate
                      pure $ updateFromCallback <<< update
                  , callback: onChange
                  }
              -- N.B. This ensures the UI doesn't lock up while the asynchronous
              -- effect is running. We allow whatever changes the user is making
              -- to go through to the state/UI immediately here, and then whenever
              -- the debounced effect completes, it will also update the state.
              -- See the function's top-level comment for additional detail.
              onChange update
        , validate
        }

Lumi has also used variants of listen that more directly capture its (unintentionally) synchronous behavior. These variants don't use an Aff indirection and so are more easily understood at-a-glance that they are synchronous. interactWith in particular also supports diffing old state and new state to determine if a synchronous response is needed in the first place:

-- | Listens for changes in a form's state and performs additional state changes
-- | synchronously. The user-specified state update function is applied to every
-- | state change made by the wrapped form.
-- |
-- | If effectful behavior or the ability to diff the old state and current
-- | state is needed, see `interactWith`.
interact
  :: forall ui props unvalidated result
   . (unvalidated -> unvalidated)
  -> FormBuilder' ui props unvalidated result
  -> FormBuilder' ui props unvalidated result
interact update = interactWith \_oldState _newState -> pure update

-- | Listens for changes in a form's state and allows for performing synchronous
-- | effects and additional state changes.
-- |
-- | The synchronous effect is run on every state change and so it must return
-- | promptly. Using the effect to change the form's state, setting some
-- | separate React state, etc. are appropriate but making network calls should
-- | be avoided. The user-specified function to build the effect is given the
-- | previous state and the current state, which enables computing a diff to
-- | determine whether or not a synchronous response to the state change is
-- | warranted. If no response is necessary or if only the response's side
-- | effecting is meaningful, the effect should return `identity` so that the
-- | form's state goes unmodified.
-- |
-- | If no effectful/diffing behavior is needed (e.g. all that's needed is a
-- | pure state update), see the simpler `interact` function.
interactWith
  :: forall ui props unvalidated result
   . (unvalidated -> unvalidated -> Effect (unvalidated -> unvalidated))
  -> FormBuilder' ui props unvalidated result
  -> FormBuilder' ui props unvalidated result
interactWith buildSynchronousCallback (FormBuilder form) =
  FormBuilder \props state ->
    let { edit, validate } = form props state
     in { edit: \(onChange :: (unvalidated -> unvalidated) -> Effect Unit) ->
            edit \(update :: unvalidated -> unvalidated) -> do
              callback <- buildSynchronousCallback state $ update state
              onChange $ callback <<< update
        , validate
        }

While listenWith and interact* are general-purpose functions, it's not clear if they're so general-purpose that its warranted to include them in purescript-lumi-components. I'm including them here in full though in case there is interest from the community to push them into the repo. Additionally, a case could be made for listen either being removed or its implementation replaced to something much closer to interactWith (no Aff).

The debouncer code is available in a gist here, and should likely also be folded into this repo if there's interest in listenWith itself making it into the repo.