ngrx / core

Core functionality for the ngrx platform
MIT License
72 stars 25 forks source link

[Investigation] Improve Reactive Forms Experience #12

Open MikeRyanDev opened 7 years ago

MikeRyanDev commented 7 years ago

A number of developers have asked if the ngrx organization would be willing to develop and maintain a reactive forms library for Angular.

Please use this ticket to report either A) usability issues with the existing reactive forms API found in @angular/forms, or B) present use cases that are not covered by @angular/forms.

Consider this step one in the design process of @ngrx/forms.

cc @DzmitryShylovich @brandonroberts @fxck @jhuntoo

fxck commented 7 years ago

Thanks for opening this. I'll get to it over the weekend. I have to refresh my memory for all the situations I encountered that I thought ngrx/forms would have helped with.


issues

  currentText$ = this.post$
    .filter(res => !!res)
    .switchMap(post => this.postContentForm 
      .get('text').valueChanges // this is actually filled using data from `this.post$`!!
      .startWith(post.text)
      .distinctUntilChanged()
    );
this.someValue$.subscribe(value => this.someForm.patchValue(value))

to fill in data simply doesn't feel reactive enough and comes with its own problems, like that when it for example receives new data that for example websocket updated, while you are in middle of editing.. there are ways to deal with it, but by having the current values inside store, it would be much easier(I think?).

questions I asked myself when thinking about how to implement ngrx/forms

MikeRyanDev commented 7 years ago

@fxck I'm also doing a lot of form intensive development over the next month. Let's capture as many issues here in that time period and work on a design for potential solutions after that.

fxck commented 7 years ago

@MikeRyan52 added couple of things I remembered and couple of question I had written down from a couple of months ago

abdulhaq-e commented 7 years ago

Hello everyone.

@fxck That is a good list of issues to think about.

Since ngrx tools combines rxjs and redux, I reviewed the redux side of the upcoming @ngrx/forms by reading the docs of redux-form which seems to be popular in the React community (http://redux-form.com/). I then made a gist in which I attempted to write pseudo Angular code that imitates the way redux-form work. Of course, the core of redux-forms was left as "magic" because the code is heavily based on React. That core I believe is where Rxjs will come into play for @ngrx/forms.

Here is the gist: https://gist.github.com/abdulhaq-e/b0abc305c16f6c14b499ccfcd56459ed

MarkPieszak commented 7 years ago

@MikeRyan52 Excited about this idea as well! If there's anyway I can help chip in, let me know!

bortexz commented 7 years ago

Would love to see this!

philjones88 commented 7 years ago

Adding interest was redirected here by an issue I added to store.

Happy to help if I can.

adharris commented 7 years ago

Complicated forms are the major pain point for my app, and it's most important feature (case management for schools, hundreds of fields across many forms, with complex conditional display and validation). While ngrx has been a boon for all other parts of my app, it hasn't helped much with my forms, which remain a highly stateful, custom, and untested mess.

I can best sum up my pain like this: It's difficult (and maybe silly) to put angular's reactive form state into the store, but I many times need to extend that state with additional data in order to render my view. I.e disable and hide this field when a second field has a specific value; change the options of the 'State/Province' field when 'Country' is changed...

I'd be very interested in helping create and using a solution that allows writing the rules to my forms be as simple and testable as ngrx makes selecting data for the view.

fxck commented 7 years ago

I think @MikeRyan52 is currently a bit busy with work, what would help is if you all who said would be interested in this project described your use cases or issues you have with @angular/forms(unless they are 1:1 with what someone else already wrote). The more the better.

tashoecraft commented 7 years ago

@fxck Your writeup had a lot of my pain points but I will try to add some more, with the main focus being the reactive forms section.

The default @angular/forms library essentially creates it's own state of the formGroup/array where the status and validity of your form lives. So if you want to put any of this into the store, you're working with statusChanges and valueChanges, submitting actions for each and reducing them. But the form will always be referencing it's own controllers, not the state. So the truth isn't in the store, but in the formGroup.

I haven't found a way to use 3rd party components built in angular (angular material, clarity, etc) because they aren't build with the idea of putting relevant information into the store. I've pretty much given up on full featured solutions and just implemented the individual ones I need, and using semantic to style. If a 3rd party component library was built off of ngrx/forms, with the ability for it to work for projects not using ngrx, then ngrx can get a big push with having the broader angular community working with it rather then creating incompatible packages (this idea is fully thought out, maybe I'll try to code something up this weekend).

Forms is definitely a struggle point when implementing ngrx for a form intensive app. I was planning on building something like a ngrx/forms, but work has taken too much of the time.

fxck commented 7 years ago

I haven't found a way to use 3rd party components built in angular (angular material, clarity, etc) because they aren't build with the idea of putting relevant information into the store.

Tell me about it, the concept of stateless components seem to be alien to almost everyone in the angular world. I'm trying to get ngrx/store friendly dialog component in material2 as we speak(https://github.com/angular/material2/pull/2853#discussion_r98515217). Dialog is one of the components where it makes the biggest sense, but it would be handy in other components as well, like the sidenav(https://github.com/angular/material2/tree/master/src/lib/sidenav) for example.

JBusch commented 7 years ago

Sounds great!

born2net commented 7 years ago

Thanks guys for raising the issue, I find myself doing a lot of busy work to connect and ngrx to the form and would love to assist in this new path... I am wondering what is the current best practice until a full blown solution is adopted? Is this what users are doing in the meanwhile ?

@Component({
  template: `<router-outlet [form]="form" [data]="data$ | async"></router-outlet>`
})
class SmartComponent {
  public data$ = this._store.let(someSelector$());

  public form = new FormGroup({
   foo: new FormControl(''),
   bar: new FormControl('')
  });

  constructor(private _store: Store<IState>) {}
}

@Component({
  template: `
    <form [formGroup]="form">
      <input type="text" formControlName="foo" />
      <input type="text" formControlName="bar" />
    </form>
  `
})
class DumbComponent {
  @Input()
  public form: FormGroup;

  @Input()
  set data(v) {
    if (v) {
      this.form.patchValue(v);
    }
  };
}

Regards,

Sean

abdulhaq-e commented 7 years ago

@born2net That's exactly what I do (except that I haven't seen data passed to `routerOutlet before) and I tend to use ng2-dynamic-forms to generate the form.

born2net commented 7 years ago

ok tx!

sebelga commented 7 years ago

Hi, Happy to see I am not the only one having doubt on how to best deal with form data coming from the store. :) And having to put back to the store the data from the forms.
Here is how we deal with the problem, I would really appreciate advice is the solution does not seem to adhere to the redux way of doing :)

@Component({
    selector: 'my-component',
    template: `
        <form [formGroup]="userForm" (ngSubmit)="saveUser()">
            <input formControlName="name"> 
            <input formControlName="lastname"> 
            <button type="submit" >Save</button>
        </form>
    `
})
export class MyComponent implements OnInit {
    private user: User;

    constructor(private store: Store<State>) {}

    ngOnInit() {
        this.store.select(stateSelector)
                    .take(1)
                    .subscribe((user) => this.user = user);

        this.userForm = this.fb.group({
            name: [this.user.name, Validators.required],
            lastname: [this.user.lastname, Validators.required],
        });
    }

    // Called on submit to update the store
    saveUser() {
        this.store.dispatch({type: 'UPDATE_USER', payload: this.userForm.value})
    }
NetanelBasal commented 7 years ago

I think it will be easier to assist if someone will write a design document for the requirements.
We can also learn from angular-redux/forms.

fxck commented 7 years ago

related http://stackoverflow.com/questions/43217078/angular-redux-ngrx-state-update-vs-forms

the-destro commented 7 years ago

Hi, I was wondering what movement there was on this and how to best help?

tashoecraft commented 7 years ago

Do we have any more info on some basic design specs? I know @fxck said he saw an early document of it. If anyone has started something, or atleast has spent some time thinking through the design specs I'd love to get started working on it. I just know that this is quite large and there's a lot of parts that can go into it. So if we have a design spec to build off of to get a MVP, then maybe this community can start building out the pieces.

nicolae536 commented 7 years ago

Any news regarding this ? Right now I'm moving my application to ngrx and I really have troubles keeping the form state especially with DatePicker control for example the date picker input allows the used to insert values and it sends the value using onChangeCallback however this will trigger my store update and the store will update the component right when the user is typing, I will be willing to help with the implementation of reactive forms for ngrx I just don't have a good idea about how to use them with material design component, I need to decuple value change from write value some how. cause right now my date picker emits a value the store gets a new state the new state updates the form model and so on. Right now for any form I have I'm not able to create new state I mutate the state for that specific form which does not respect the over all pattern. Anyone has time to start a implementation I could help.

mfp22 commented 7 years ago

What's happening with this? I am very intersted.

otienoanyango commented 7 years ago

I found this approach interesting. Still needs a little tweaking to fit exact needs but is a great starting point. https://netbasal.com/connect-angular-forms-to-ngrx-store-c495d17e129

fxck commented 7 years ago

Not really. It doesn't really solve any of the problems mentioned in this issue.. The problem is that angular forms are "flawed" from the beginning.

marcelnem commented 7 years ago

Is there currently a recommendation / best practice on how to go about forms in angular with ngrx and one-way data binding? Is it currently better to use template driven angular forms or reactive angular forms with ngrx? I understand that this thread is searching for a solution close to optimal, but what is a recommended solution until we get there? Eventually, is it better to not use ngrx with forms for now and only use it for other parts of the application? Or keep using two-way data binding and update store somehow? I used React/Redux before. I am new to Angular, trying to use Angular2 + ngrx for our enterprise project.

fxck commented 7 years ago

Reactive forms are always better since they are.. reactive. Recent changes in angular forms make it a little easier to just create some sort of bridge, but I still think ngrx/forms would be a better solution. I couldn't quite get in touch with @MikeRyanDev to check what's their latest opinion on this.

szebrowski commented 7 years ago

Recent changes in angular forms make it a little easier to just create some sort of bridge

can you write a little about what changes you are talking? :)

marcelnem commented 7 years ago

So I understand that reactive Angular forms are recommended to be used with ngrx (instead of template-driven Angular forms). The recommendation comes from @fxck two comments up.

Now the question is how to use reactive Angular forms with ngrx. For angular-redux there is angular-redux/forms with a brief description on how it enables us to keep forms state in redux store.

I found this (not tried yet) article describing how to use reactive Angular forms with ngrx: https://netbasal.com/connect-angular-forms-to-ngrx-store-c495d17e129

Would you recommend using the approach or are there any other approaches/guides?

sdargaud commented 7 years ago

@marcelnem The link you posted may not solve all your problems. For example, it initializes the form from the store, but any subsequent change will not be taken into effect (the .take(1)). I haven't tested without it but you may face circular calls (i.e. you update the store, which updates the form, which updates the form etc.). Has anyone tackled this problem ?

fxck commented 7 years ago

@szebrowski https://github.com/angular/angular/pull/18577 and afaik there's something in works for https://github.com/angular/angular/issues/6895 as well (https://docs.google.com/document/d/1dlJjRXYeuHRygryK0XoFrZNqW86jH4wobftCFyYa1PA/edit#heading=h.r6gn0i8f19wz)

@marcelnem there's currently no lib that syncs forms state to the store, which is what this issue was created for.. what is described in that article barely scratches the surface.. the only advantage reactive forms over template driven forms is that they provider observable state changes, so it makes them easier to map to ngrx observables

MrWolfZ commented 7 years ago

I've been working in a project that makes heavy use of forms. Since there was no proper solution available I've built my own version of @ngrx/forms (or rather I built two since version 1 was only a rough first draft trying to re-use reactive forms).

The project is very much still WIP, but I'd be happy if some of you could have a look and tell me your thoughts of whether this has any merit. Since I didn't get around to documenting this properly yet, I'll quickly outline the current state and open points below.

The general idea is to have the complete form state in the store, including all the flags like isDirty etc. Updates to the state therefore happen fully through reducers (which makes testing a breeze). There is one single directive so far, applied via [ngrxFormControlState]. With this directive I am completely side-stepping the whole template or reactive forms and I am only using some basic infrastructure of @angular/forms, namely the ControlValueAccessor mechanism. The directive is responsible for dispatching the proper actions to update the state. And of course it is always possible to manually update the state by dispatching actions on your own.

The reducers automatically recompute the whole state tree, i.e. if a parent group is disabled, all children get disabled, if a child is marked as dirty, all parents are marked as dirty etc. The group reducer also dynamically creates and removes child states based on the current value of the group which makes it very easy to add and remove controls. Have a look at the reducer.spec.ts to see all the scenarios covered.

The two missing things are:

Please let me know your thoughts.

Usage examples:

reducer.ts

import { createFormGroupReducer } from '@ngrx/forms';

export interface MyFormValue {
  someTextInput: string;
  someCheckbox: boolean;
  nested: {
    someNumber: number;
  };
}

export const FORM_ID = 'some globally unique string';

export const myFormReducer = createFormGroupReducer<MyFormValue>(FORM_ID, {
  someTextInput: '',
  someCheckbox: false,
  nested: {
    someNumber: 0;
  },
});

component.ts

import { FormGroupState } from '@ngrx/forms';

@Component({ ... })
export class MyComponent {
  formState$: Observable<FormGroupState<MyFormValue>>;

  constructor(private store: Store<RootState>) {
    this.formState$ = store.select(s => s.path.to.form.state);
  }
}

component.html

<form novalidate>
  <input type="text"
         [ngrxFormControlState]="(formState$ | async).controls.someTextInput">

  <input type="checkbox"
         [ngrxFormControlState]="(formState$ | async).controls.someCheckbox">

  <input type="number"
         [ngrxFormControlState]="(formState$ | async).controls.nested.controls.someNumber">
</form>

module.ts

import { NgrxFormsModule } from '@ngrx/forms';

@NgModule({
  imports: [
    NgrxFormsModule,
  ],
})
export class MyModule{ }
MrWolfZ commented 7 years ago

Alright, I've implemented form submission tracking. I've also created an example application that showcases how the library manages form states.

MrWolfZ commented 7 years ago

So, I've finally gotten around to releasing v1.0.0 of ngrx-forms. You can get the library on npm via

npm install ngrx-forms --save 

See the readme for a detailed user guide.

johnchristopherjones commented 7 years ago

I'm surprised nobody has mentioned this, so perhaps I'm missing something. If you want Store changes to update the form, I don't think can use any FormArrays with ReactiveForms.

The problem is that to you need to bind outgoing changes from the form to store events, and incoming store changes to patchValue the store. To prevent an infinite loop, you must use patchValue(formModel, {emitEvent: false}) to prevent the patch from emitting another valueChange.

However, to update a FormArray, you have to use setControl. I haven't been able figure out how to either disable setControl from emitting a valueChange or a reliable way to filter out the offending event. This could be fixed by either adding an options param to setControl give it parity with patchValue or make patchValue work on FormArray.

james-schwartzkopf commented 7 years ago

To prevent an infinite loop, you must use patchValue(formModel, {emitEvent: false})

Careful with that, we found it causes issues with material2 components (placeholders don't move, etc). We ended up creating a diff object between the store and the current form values and then (if it's not empty) using that with patch value and emitEvent true.

jayordway commented 6 years ago

Has this been abandoned? I have a use case as to where there is a need for many large - very large - reactive forms. The composition or content of these forms changes dynamically depending on the user's roles and entitlements. I'd prefer to handle this within the reducers - reactive forms and ngrx don't still mix well yet though, correct?

fxck commented 6 years ago

The discussion moved to https://github.com/ngrx/platform/issues/431