Skip to content
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.

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

asyncSetValidate giving partial output #48

Closed
naglalakk opened this issue May 26, 2019 · 9 comments
Closed

asyncSetValidate giving partial output #48

naglalakk opened this issue May 26, 2019 · 9 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@naglalakk
Copy link

naglalakk commented May 26, 2019

Environment

  • purescript-halogen v5.0.0-rc.4
  • purescript-halogen-formless halogen-5 (branch)

Current behavior

When I use asyncSetValidate with a debounce time the output from validation seems to chop off a bit of the end depending on how fast I type. Entering something like john@john.com quite fast gives -> john@jo etc.

This could of course simply be an issue with my code but it works well if I have no debounce time. Could it be that if I have debounceTime 300.0 for example and then validation kicks off after that time, the continuing input is disregarded until the validation function is finished?.
Because in my case it seems that I type something, and it valdiates a part of it and then it doesn't validate it again unless I trigger it again by writing one extra letter, then it validates the whole thing.

(Screenshot where debounceTime = 0.0 Milliseconds)
Screen Shot 2019-05-26 at 00 54 19

(Screenshot where debounceTime = 500.0 Milliseconds)
Screen Shot 2019-05-26 at 00 56 32

Code being used

(spec)

      , HH.input
          [ HP.value $ F.getInput _email form
          , HE.onValueInput $ Just <<< F.asyncSetValidate debounceTime _email
          ]
  
    where
    _email = Sproxy :: Sproxy "email"
    debounceTime = Milliseconds 500.0

(validation)

checkIsInUse :: forall form m r
              . MonadAff m 
             => MonadAsk { logLevel :: Environment, baseURL :: BaseURL, apiURL :: BaseURL | r } m
             => F.Validation form m EmailError String Email
checkIsInUse = F.hoistFnME_ $ \email -> do
  apiUrl <- asks _.apiURL
  let auth = Nothing
  let endpoint = "/users/exists/" <> email
  let opts = { endpoint: endpoint, method: Get } :: RequestOptions
  res <- liftAff $ AX.request $ defaultRequest apiUrl auth opts
  case res.body of 
    Left err -> do
      pure $ Left Error
    Right json -> do
      case (J.stringify json) of
        "true"  -> pure $ Left Exists
        "false" -> pure $ Right $ Email email
        _       -> pure $ Left Error

Expected behavior

The asyncSetValidate should validate all Input

@thomashoneyman thomashoneyman self-assigned this May 26, 2019
@thomashoneyman thomashoneyman added bug Something isn't working help wanted Extra attention is needed labels May 26, 2019
@thomashoneyman
Copy link
Owner

Hi @naglalakk! I'm sorry you're having this issue, and thank you for bringing it to my attention.

Formless is designed to cancel running validation code when new input is received and not re-run validation until the debouncer has been triggered again. The relevant code is here:

https://github.com/thomashoneyman/purescript-halogen-formless/blob/master/src/Formless/Internal/Debounce.purs

I suspect you are correct and the validation function is running while you are typing new input. That would allow the new input to still update in state correctly (there is no lost text in your text field, for example), but for the debouncer to malfunction.

I'm in quite a busy period at work & school at the moment so I may not be able to solve this issue right away. It's quite a serious bug, however, so it needs to be fixed. I'll try to not make you wait too long for a solution, but if you need this fixed urgently I would suggest looking at that debouncer file first.

@naglalakk
Copy link
Author

Hi @thomashoneyman thank you so much for your reply and no worries. I just wanted to post this here to bring it to your attention if it was a bug. I'm quite new to halogen/purescript in general so I thought maybe I'm just doing something wrong. I can totally work around this and for now I'm working on/testing applications with such small load that debounceTime is not vital. Thanks for addressing this so quickly 👍

@thomashoneyman
Copy link
Owner

@naglalakk To help narrow this down, do you mind answering these questions?

  • How often does this behavior appear? Every time you trigger the debouncer, validation begins, and you begin typing again? Or just every once in a while?
  • What happens if you use the Validating status to render some text that says 'validating'? Does the text get stuck in that 'validating' state even while you're typing new input? You can see this text using the FormFieldResult type and you can get that out of your form using getResult.

If you have the time, these could also be useful:

  • What happens if you extend or reduce the debounce time (not to 0)? Are you still able to trigger it?
  • What happens if you extend the validation function's time (for example: this delayed validation function)?

@naglalakk
Copy link
Author

@thomashoneyman I would be more than happy to help! Things are quite busy atm but i'm going to sit down with this as soon as possible and report back.

@naglalakk
Copy link
Author

Hi sorry for a late reply I've had some busy days. Here are my results

How often does this behavior appear? Every time you trigger the debouncer, validation begins, and you begin typing again? Or just every once in a while?

When validating with asyncSetValidate this seems to happen when I enter multiple characters in a row. Once the debouncer is finished it does not trigger validation for the rest of the data unless another character is entered.

Say we have debounce time 500.0

typing:

test@testcompany

       ^
       |
        - here I start typing faster
       |
       500.0
       |
        - validated result = test@tes 
        - validation does not trigger again unless I enter another character

This usually isn't an issue when user is still typing then validation will simply be triggered again once the debouncer has finished but this cuts off the end of strings. For
example

test@test.is = test@tes
   |   |
   |   Validation triggered -> test@tes
   |
  Validation triggered -> test

What happens if you use the Validating status to render some text that says 'validating'? Does the text get stuck in that 'validating' state even while you're typing new input? You can see this text using the FormFieldResult type and you can get that out of your form using getResult.

The status shows "Output" when typing. When I stop typing I see it switch to "Validating" for a split second. It does not seem to get stuck in Validating.
But looking at the status it seems that once it starts validating it's validating the input it got from before I started typing again. This also happens when erasing characters. For example

test@test.is

(delete 3)

test@test = test@test.i

What happens if you extend or reduce the debounce time (not to 0)? Are you still able to trigger it?

The method always gets triggered. I've tried with debounceTime = 0, 20, 300, 500, 1000. The time passing always seems correct.

What happens if you extend the validation function's time (for example: this delayed validation function)?

This works perfectly. I set debounceTime on asyncSetValidate to = 0.0 and added a 500ms delay to the function itself and now the server is only hit every 500ms and including every letter I enter.

@skress
Copy link
Contributor

skress commented Jan 23, 2020

I think there are currently two problems. You can check it on the example site https://thomashoneyman.github.io/purescript-halogen-formless/#async.

Problem 1: Copy a valid email address into your clipboard. Start with a blank form in the "async" example. Enter a character into the email address field (e.g. 'v') and quickly paste the valid email address into the field. Although there is a valid email address in the input field the "The email is not valid" error is shown.

The reason for this problem is as follows Formless.Component contains this code for modifyValidate:

  , modifyValidate: \(Tuple milliseconds variant) -> do
      let
        modifyWith
          :: (forall e o. FormFieldResult e o -> FormFieldResult e o)
          -> HalogenM form st act slots msg m (form Record FormField)
        modifyWith f = do
          st <- H.modify \s -> s
            { form = IT.unsafeModifyInputVariant f variant s.form }
          pure st.form

        validate = do
          st <- H.get
          let vs = (unwrap st.internal).validators
          form <- H.lift do
            IT.unsafeRunValidationVariant (unsafeCoerce variant) vs st.form
          H.modify_ _ { form = form }
          pure form

      case milliseconds of
        Nothing ->
          modifyWith identity *> validate *> handleAction handleAction' handleEvent sync
        Just ms ->
          debounceForm
            ms
            (modifyWith identity)
            (modifyWith (const Validating) *> validate)
            (handleAction handleAction' handleEvent sync)

modifyWith is provided to the Formless.Internal.Debounce component. Upon the first invocation the debouncer reference contains Nothing. Now the debouncer gets initialized and also this code gets forked:

_ <- H.fork do
        void $ H.liftAff (AVar.take var)
        H.liftEffect $ traverse_ (Ref.write Nothing) dbRef
        atomic post (Just last)

Now, whenever new input is typed into the field modifyValidate gets called again. In this case the debouncer is already there, i.e. another code path is taken. This one just kills the fiber and creates a new one to debounce the validation.

When no new input is entered the AVar gets filled and the above AVar.take finally succeeds. Now atomic post (Just last) gets called. post is the local function modifyWith from Formless.Component that captured the input via the variant parameter. As this post value is the one that was created right after the first character was typed, this is the value that gets used now instead of the latest input value.

I have a fix for this problem and will send a PR soon.

Problem 2: Again the "async" example. Start with a blank form. Enter a number into donation field and quickly type one character into the name field. Now wait until the donation validation is finished. You will notice that the whatever you typed into the name field gets deleted after the validation of the donation finishes.

I think the problem here is the local validate function in modifyValidate

validate = do
          st <- H.get
          let vs = (unwrap st.internal).validators
          form <- H.lift do
            IT.unsafeRunValidationVariant (unsafeCoerce variant) vs st.form
          H.modify_ _ { form = form }
          pure form

This function accesses the component's state and provides it as a parameter to IT.unsafeRunValidationVariant. The provided validation itself can be run async just like in the example when checking the value of the donation. So the form state might already have been changed when the validation code finally runs and then current st.form is overwritten with a value that's derived from the old value.

I haven't looked deep enough into the code to come up with an idea of a fix ...

There might be a third problem: Start with a blank form in the "async" example. Now enter "1" into the donations field and quickly after that any character into the "email" field. The donation field will now get never past the "validating ..." state. I haven't looked into that - so it might get fixed when the first two problems are fixed.

@skress
Copy link
Contributor

skress commented Jan 24, 2020

I have updated the PR to fix the second problem mentioned above.

There is still the third problem: when the debouncer is used for one field and then for another field the running validation for the first field might get killed in atomic in Internal.Debounce:

  atomic
    :: forall n
     . MonadAff n
    => HalogenM form st act ps msg n (form Record FormField)
    -> Maybe (HalogenM form st act ps msg n a)
    -> HalogenM form st act ps msg n Unit
  atomic process maybeLast = do
    state <- H.get
    let ref = (unwrap state.internal).validationRef
    mbRef <- readRef ref
    for_ mbRef H.kill
    H.liftEffect $ for_ ref $ Ref.write Nothing
    forkId <- H.fork do
      form <- process
      H.modify_ _ { form = form }
      H.liftEffect $ for_ ref $ Ref.write Nothing
      for_ maybeLast identity
    H.liftEffect $ for_ ref $ Ref.write (Just forkId)

Not sure what the best approach for a fix would be. Obviously there could be a debouncer per form field or only the forked validation (in atomic) could be run per form field.

@thomashoneyman
Copy link
Owner

@skress My gut sense is that you're right and we'll ultimately need to enable the async validation per field. It's been a little while since I've visited how the validation is applied, but validation is done on a per-field basis so I expect debouncing should be possible on a per-field basis as well.

@thomashoneyman
Copy link
Owner

This is closed by #63, and I'm opening a new issue for per-field debouncers. Thanks @skress!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants