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

Reactive forms refresh #201

Closed
1 of 4 tasks
Jordan-Hall opened this issue Nov 30, 2020 · 16 comments
Closed
1 of 4 tasks

Reactive forms refresh #201

Jordan-Hall opened this issue Nov 30, 2020 · 16 comments
Labels

Comments

@Jordan-Hall
Copy link

Summary

I'm submitting a:

  • bug report
  • feature request
  • question / support request
  • other

When using the component inside reactive forms I've notice a bug. When you refresh the page the component has a valid token but, not showing it it ticked

@DethAriel
Copy link
Owner

Hi @Jordan-Hall, and thank you for reaching out!

When you say "refresh the page" -- what exactly do you mean, can you elaborate? How are you getting the "valid token" from the component?

@Jordan-Hall
Copy link
Author

So i completed the form, captcha. I then refresh the website. The captcha apparently still contains a value but not ticked

@DethAriel
Copy link
Owner

@Jordan-Hall

The captcha apparently still contains a value but not ticked

What makes you think that captcha still contains a value? reCAPTCHA value is only ever stored in memory, and is not preserved upon page reloads

@Jordan-Hall
Copy link
Author

@DethAriel because when i debugged the code it still had a value in the reactive form

@DethAriel
Copy link
Owner

@Jordan-Hall can you come up with a repro code sample and step-by-step instuctions? This behavior certainly sounds unexpected, but unfortunately I can't reproduce myself.

@mokipedia
Copy link

mokipedia commented Jan 19, 2021

I am seeing the same issue with reactive form. I think what @Jordan-Hall wanted to say is not "reload" the page but destroy the recaptcha component.
At least for me, having the component in an ngIf does exactly what @Jordan-Hall described - the captcha is not ticked anymore but the formcontrol is still valid and filled with the previous value. clearing the formcontrol by yourself also breaks the view.
here is a stackblitz with a ngIf on a timer to show what is going on:
https://stackblitz.com/edit/angular-ivy-htkcc9?devtoolsheight=33&file=src/app/app.component.ts

My workflow:

  1. form gets filled, also captcha is valid
  2. form gets submitted, spinner is shown instead of form
  3. form submit ended (with or without error does not matter)
  4. form says it's valid, captcha value still there, but check is gone

@mokipedia
Copy link

up

@mokipedia
Copy link

a workaround for this issue (at least mine, @Jordan-Hall if what i describe is not your issue, pls tell me and I will open up another issue) is to not use formValueAccessor and instead use the resolved event to manually patch the form. That way I can at least reset the formControl to be aligned with the recaptcha component

@DethAriel
Copy link
Owner

Thank you very much for the reproduction scenario @mokipedia! I can now reliably reproduce the behavior.

At this point I'm still on the fence whether to treat this as a bug, and if so - whether to fix it or not. The value provided as reCAPTCHA responses are supposed to be single-use only, and after grabbing it and passing it to the server, the reasonable expectation is that developers will reset the <re-captcha> component. However, the behavior demonstrated by your repro scenario does feel counter-intuitive, hence I'm undecided still. While I'm trying to figure out whether this warrants a fix, I want to call out that doing so would likely involve a component breaking change.

If you (or anyone else) are looking for a way to avoid this unfortunate situation, my recommendation would be to manually reset <re-captcha> component after using its value, something like this:

public onFromSubmit() {
  this.tryLoginUser(this.formGroup.value)
    .subscribe(() => {
      // if success, probably navigate away
      this.navigate();
    }, (error) => {
      // if error, and reCAPTCHA challenge needs to be presented again, reset its value
      this.formGroup.setValue({ captcha: null });
    });
}

@mokipedia
Copy link

mokipedia commented Jul 22, 2021

@DethAriel have you tried that? I tried to patch or set value, in that case your formvalueaccessor screamed at me that this is not allowed. (Haven't tried since writing my workaround, so maybe it changed?)

@DethAriel
Copy link
Owner

@mokipedia yes, this does result in a console error from the value accessor (which is another avenue for a fix), however this error doesn't really affect the app in any detrimental way

@mokipedia
Copy link

@DethAriel I would agree not fixing this might be the way to go as long as you allow a reset without error (e.g. with control value accessor)

However doing a reset like you suggest actually breaks logic in a form I am using it, hence the workaround. Not sure what exactly happens but the form is unusable afterwards.

@DethAriel
Copy link
Owner

@mokipedia understood, thanks for working through this issue with me. I think I have an idea of how to fix this now, so I'll be working on a code update. What ng-recaptcha version are you on? I would like to better understand the extend of fix back-porting that I'll need to go through

@mokipedia
Copy link

7.0.1 at the time of first noticing the error due to a code refactor on our side.

@DethAriel
Copy link
Owner

The error that is generated when one is performing this.formGroup.setValue({ captcha: null }); has been fixed in Angular 11.1.0, it's this one:

forms: clean up connection between FormControl/FormGroup and corresponding directive instances (#39235) (a384961), closes #20007 #37431 #39590

While there is a lib-specific workaround I can implement, I'd rather not go that route. Especially considering that a fix for this issue that I'll back-port to 7.x.x branch will no longer require you to clear the form value manually.

DethAriel added a commit that referenced this issue Jul 22, 2021
@DethAriel
Copy link
Owner

This has now been fixed and released in v7.0.2 (for Angular 11) and v8.0.1 (for Angular 12). You can observe the new behavior here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants