Perhaps better way to update ngrx on form.valuechanges()

BBX999 commented 4 years ago

I noticed in the current form.component.html the following code is used in the template to update the ngrx model:

<ng-container *ngIf="formValueChanges$ | async as updatedForm">
    {{update(updatedForm)}}
</ng-container>

I found that this triggers multiple and duplicate actions for even a single key entry on an input field. It also triggers the action on clicks within the form when data has not changed. I am wondering if this is by design or whether the duplicate actions are tolerated for the sake of the automatic unsubscribe by using the | async pipe.

If the duplication is tolerated for the sake of the automatic unsubscribe, I think I have a possible improvement. Instead of a function call within the template, the template would just contain the subscription:

<ng-container *ngIf="formValueChanges$ | async as updatedForm">
</ng-container>

but the component would provide for the action to be triggered in a tap operator of the formValueChanges$ observable:

ngOnInit() {
    this.formValueChanges$ = this.form.valueChanges.pipe(
      debounceTime(500),
      filter((form: Form) => form.autosave),
      tap(updatedForm => update(updatedForm))
    );
}

This seems to avoid duplicate actions. Thoughts? Any potential drawbacks to this solution?

millbj92 commented 2 years ago

Submitted a PR for this, btw. Nice catch man!

BBX999 commented 2 years ago

Thanks!

On Sun, Aug 8, 2021 at 3:30 AM Brandon Miller @.***> wrote:

Submitted a PR for this, btw. Nice catch man!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tomastrajan/angular-ngrx-material-starter/issues/527#issuecomment-894723042, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE5W3GGE7XHMQNGSYHEHKLDT3XFZZANCNFSM4NDBWETQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

tomastrajan / angular-ngrx-material-starter

Perhaps better way to update ngrx on form.valuechanges() #527