feat(form-plugin): add reset form action

[ozanturhan]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/ngxs/store/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

ResetForm action added for form plugin.

[splincode]

@ozanturhan update docs please, also

[ozanturhan]

Hello @splincode I updated form.md is it enough?

[markwhitfeld]

@ozanturhan Thank you for your PR. Congratulations on your first one to NGXS!
Would you mind providing the motivation for this change in the description for the PR.
ie:

• What is the use case?
• What code would you have to write to achieve the same if this feature didn't exist?
• How does this make things simpler or cleaner?

This will help us to decide the value that this feature brings.
Thank you 😄

[ozanturhan]

@markwhitfeld Thanks a lot for your feedback.

I want to reset the form on the related page whenever the parameter changes in the resolver. I'am using the "UpdateFormValue" and "UpdateFormDirty" actions for this. But i couldn't reset form without passing all form members. You can compare the following two usages.

An example from our project:

  resolve() {
    return this.getSearchConfig().pipe(
      switchMap(() =>
        this.store.dispatch([
          new UpdateFormValue({
            value: {
              type: null,
              status: null,
              categoryId: null,
              coverage: null,
              commitmentEndDate: null,
              phoneNo: null,
              firstName: null,
              lastName: null,
              deviceType: null,
              brand: null,
              deviceCharModel: null,
              deviceCharCapacity: null,
              deviceCharColor: null,
            },
            path: 'ServiceState.searchForm',
          }),
          new UpdateFormDirty({ dirty: false, path: 'ServiceState.searchForm' }),
        ]),
      ),
      mapTo(null),
    );
  }

Example with ResetForm action:

  resolve() {
    return this.getSearchConfig().pipe(
      switchMap(() =>
        this.store.dispatch(
          new ResetForm({ path: 'ServiceState.searchForm' }),
        ),
      ),
      mapTo(null),
    );
  }

[markwhitfeld]

Thank you for your motivation! I think it does make sense as a feature we need.
See the requested changes below.

[markwhitfeld]

Ok, I think that I need to get a deeper understanding from you as to what all the concerns are here.
I last worked with this plugin's code more than a year ago.
Could you explain what your expectations of this ResetForm action would be with regard to:

• The final value that the model, status, dirty, errors, and disabled properties would be in the state.
• The timing of how things would be set in relation to the debounce timeout
• Any other actions that may get fired by NGXS as a result of dispatching this action
• Any observable events that the form would publish as a result of this action
• Anything else that I would need to understand?

[markwhitfeld]

If you could answer these questions with tests in this PR, I would be really impressed ;-)

[ozanturhan]

@markwhitfeld Thank you for your comments. I will be working on this weekend.

[ozanturhan]

Hi @markwhitfeld,

I have a question. I couldn't decide what it should be.

What should be expectation in below test case?

    await store.dispatch([
      new UpdateForm({
        errors: { something: true },
        dirty: true,
        status: 'INVALID',
        value: { name: 'Ozan Turhan' },
        path: 'student.studentForm'
      }),
      new ResetForm({ path: 'student.studentForm' })
    ]).toPromise();

When we run test without form, should these fields to be reset (errors, dirty, status etc.) ? I think it shouldn't be change. Because these parameters related to form.

For example in this test case ResetForm action changes only model. Because there is no form also ngxsForm directive.

Student Form Before Reset:

{
  dirty: true,
  errors: { something: true  },
  model: {
      name: "Ozan Turhan",
  },
  status: "INVALID",
}

Student Form After Reset:

{
  dirty: true,
  errors: { something: true  },
  model: {},
  status: "INVALID",
}

But if we are run test with runtime behavior, also if we have a form with ngxsForm directive studentForm state will be like below (after reset form action called):

{
  dirty: false,
  errors: {},
  model: {
      name: null
  },
  status: "VALID",
}

Example Form:

      @Component({
        template: `
          <form [formGroup]="form" ngxsForm="todos.todosForm">
            <input formControlName="text" /> <button type="submit">Add todo</button>
          </form>
        `
      })
      class MockComponent {
        public form = new FormGroup({
          text: new FormControl()
        });
      }

I think runtime behavior is true. But I'm not sure about the first case (without ngxsDirective).

After your anwser i will be start to writing to the tests.

If you can help me to make decision for that, I would be glad.

Regards

[markwhitfeld]

Apologies for the delay in getting back to you!

This is a very interesting case that you raise.
My gut feel is that if you are not bound with an ngxsForm then the model and everything else should be reset. ie. not dirty, not touched, no errors, reset status, etc.

Conceptually, when you dispatch actions there should be no observable difference in the state if something is bound to that state or not. In this case there is a subtle difference, which essentially is that if you are bound to a form then the model resets to an object with empty properties (as described in the form) instead of an empty object ({}). I am willing to live with this because there is no way for the state to know what properties are on the form if it has never been bound.
Do you agree?

[markwhitfeld]

PS. With regard to the debounce. I think that this action should result in an immediate change to the state and the form, and that any pending changes that are being debounced should be ignored.
For example... let's say that we had a debounce of 500ms and we type into one of the inputs.
The model in the state would only be updated after 500ms as expected.
But, if we were to dispatch the ResetForm action before that 500ms was up then the input change should never reach the state.

If we do not address this then we could have this strange scenario (500ms debounce):

• user types input
• after 200ms, ResetForm is dispatched by some code
• form model and all other props are rest
• at 500ms the new value arrives in the state!
• this would not be expected

[ozanturhan]

@splincode Thanks a lot for your support.

Apologies for the delay in getting back to you!

This is a very interesting case that you raise.
My gut feel is that if you are not bound with an ngxsForm then the model and everything else should be reset. ie. not dirty, not touched, no errors, reset status, etc.

Conceptually, when you dispatch actions there should be no observable difference in the state if something is bound to that state or not. In this case there is a subtle difference, which essentially is that if you are bound to a form then the model resets to an object with empty properties (as described in the form) instead of an empty object ({}). I am willing to live with this because there is no way for the state to know what properties are on the form if it has never been bound.
Do you agree?

Yes i agree. Form state should be reset without bounding to ngxsForm. Reset action will reset dirty, status etc.

But if we have ngsxForm state will be reseting by form raw values.

I got it. I covered these cases with unit tests. I am going to commit these tests.

PS. With regard to the debounce. I think that this action should result in an immediate change to the state and the form, and that any pending changes that are being debounced should be ignored.
For example... let's say that we had a debounce of 500ms and we type into one of the inputs.
The model in the state would only be updated after 500ms as expected.
But, if we were to dispatch the ResetForm action before that 500ms was up then the input change should never reach the state.

If we do not address this then we could have this strange scenario (500ms debounce):

• user types input
• after 200ms, ResetForm is dispatched by some code
• form model and all other props are rest
• at 500ms the new value arrives in the state!
• this would not be expected

Also i added new test case for this.

[markwhitfeld]

Please remove the propertyPath because, as stated below, I don't think it makes sense in this case.
Do you agree?
Are there any other aspects you feel that we still need to discuss/cover?

PS. I really appreciate and find it really encouraging that you have engaged so well about getting this feature implemented. Often people drop a PR and are not willing to discuss or put extra effort in. You have really gone the extra mile! Your contribution is definitely a highlight in the often thankless task of open source work 😀

[ozanturhan]

Please remove the propertyPath because, as stated below, I don't think it makes sense in this case.
Do you agree?
Are there any other aspects you feel that we still need to discuss/cover?

PS. I really appreciate and find it really encouraging that you have engaged so well about getting this feature implemented. Often people drop a PR and are not willing to discuss or put extra effort in. You have really gone the extra mile! Your contribution is definitely a highlight in the often thankless task of open source work

Okay, you are right. i will remove it tomorrow. I don't think there's anything left to discuss.

Thank you very much for your positive comments.

Thank you for your interest and support in this PR. This will be my second contribution to an open source project. It was a very good experience for me. Also your comments were useful and I really learned something.

[markwhitfeld]

Thank you so much for this work. I would like to release this soon in the next version.
Would you mind adding one more thing to this PR?
Could you add a section to the announcement article with a brief overview of this feature.
Edit this file: publication/announcing_ngxs_next/article.md
This would really help out.

[ozanturhan]

@markwhitfeld of course i will add as soon as possible.

[markwhitfeld]

A few small suggested language changes and a question on a previous comment.

[nickroberts]

This is great! We are currently using the form plugin for several different forms on a single page, and resetting them is a huge pain.

Any idea when the next release will be? I was hoping this was in the latest dev version, and I was going to use that, but it doesn't look like it is, as that version was from May

Keep up the great work!