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

A form value can be an old one #98

Closed
maxime1992 opened this issue Sep 4, 2019 · 1 comment · Fixed by #99
Closed

A form value can be an old one #98

maxime1992 opened this issue Sep 4, 2019 · 1 comment · Fixed by #99
Assignees
Labels
effort-1: minutes Will only take a few minutes to fix/create released scope: lib Anything related to the library itself state: has PR A PR is available for that issue type: bug/fix This is a bug or at least needs a fix workaround: none No workaround

Comments

@maxime1992
Copy link
Contributor

This is a tricky issue to repro and will probably only repro through a test to get the timing right.

But I just hit a case that drove me insane where my form was considered valid, while the value that was passed clearly wasn't.

@zakhenry and I identified the potential issue and it might be coming from the fact that we do:

 this.subscription = this.formGroup.valueChanges
      .pipe(
        // ...
        delay(0),
        map(changes => {
          this.onChange(changes)
        }),
        // ...
      )

But apparently that value coming from the form might not be mutated like I thought it'd and thus, the validity check would succeed on the RootFormComponent because the form might now be valid while sending the previous value.

@maxime1992 maxime1992 added effort-1: minutes Will only take a few minutes to fix/create scope: lib Anything related to the library itself type: bug/fix This is a bug or at least needs a fix workaround: none No workaround workaround-2: non obvious Non obvious workaround and removed workaround: none No workaround workaround-2: non obvious Non obvious workaround labels Sep 4, 2019
@maxime1992 maxime1992 self-assigned this Sep 4, 2019
maxime1992 added a commit that referenced this issue Sep 5, 2019
@maxime1992 maxime1992 added the state: has PR A PR is available for that issue label Sep 5, 2019
@maxime1992
Copy link
Contributor Author

🎉 This issue has been resolved in version 2.11.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-1: minutes Will only take a few minutes to fix/create released scope: lib Anything related to the library itself state: has PR A PR is available for that issue type: bug/fix This is a bug or at least needs a fix workaround: none No workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant