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

Bind form fields values to actual objects #243

Open
sebastian-zarzycki-apzumi opened this issue Mar 27, 2017 · 22 comments
Open

Bind form fields values to actual objects #243

sebastian-zarzycki-apzumi opened this issue Mar 27, 2017 · 22 comments

Comments

@sebastian-zarzycki-apzumi

If I understand correctly, the current implementation of mobx-react-form is one-way binding - we can pass some initial values to the fields, but after that, the state is maintained internally within the form object and loses completely it's connection to initial object. This is less than ideal. In true object-oriented manner, that mobx is encouraging to use, one would bind directly to an @observable value. This means, I could do something like this:

@observable formData = {
        firstName: 'hai'
};

this.form = new MobxReactForm(
        {
            fields: {
                firstName: {
                    label: 'First Name',
                    placeholder: 'Type Firstname',
                    rules: 'required|string|between:5,25',
                    value: this.formData.firstName
                }
        },
        {
            plugins: { dvr: validatorjs }
        }
);

and when interacting directly with input (typing in new value), that value would automatically land back in this.formData.firstName, effectively giving us the two-way databinding.

The current solution of asking for form.values() upon submitting/validating and then copying them back to the appropriate objects seems needlessly cumbersome. Am I missing something here? Is there an easy way to achieve this? And by easy I mean other than multiple declarations of form.$('field').observe(({ form, field, change }) => { ... }); ?

@foxhound87
Copy link
Owner

foxhound87 commented Mar 28, 2017

Actually you don't need to do that, you can access the current fields values (true mobx observable/computed values) using the fields selector: form.$('firstName').value

@sebastian-zarzycki-apzumi
Copy link
Author

That's completely not the point. The point is to make form bind to a object you already have, rather than making it a disjointed set of fields, which you will then use to gather values and assemble then back again in the original object.

If I define something as value: this.formData.firstName, when I change that field in my form, let this.formData.firstName update, too.

@foxhound87
Copy link
Owner

foxhound87 commented Mar 28, 2017

That's not the intended behavior of this package. The package is a store for form and fields which are created providing an initial field structure and then provide some common forms patterns, so you need to use the stores to bind props to your components and inputs. Because the Form and Fields stores have their own internal behaviors and functionalities which you don't get if you derive further the fields values. I suggest you to use the DevTools to see all the fields stores structure with their observables and computed props.

@sebastian-zarzycki-apzumi
Copy link
Author

We still don't understand each other. All I'm saying is that it would be handy to somehow specify in field definition the target property to update whenever the form store property changes - to provide two-way databinding.

@foxhound87
Copy link
Owner

The package already have two-way data binding on each value Field property.

@foxhound87
Copy link
Owner

Maybe you need to define the observer from the field definition instead using the observe() method?

@sebastian-zarzycki-apzumi
Copy link
Author

What do you mean?

@foxhound87
Copy link
Owner

I mean, in the first message you asked about the use of observe().
You can define observers on the field definition like this:

const fields = {
  firstName: {
      label: 'First Name',
      placeholder: 'Type Firstname',
      rules: 'required|string|between:5,25',
      // define also multiple observers:
      observers: [{
        key: 'value', // can be any prop
        call: () => console.log('changed'),
      }],
  },
};

@sebastian-zarzycki-apzumi
Copy link
Author

This is somewhat more elegant, thank you. But it's not directly documented in https://foxhound87.github.io/mobx-react-form/docs/api-reference/fields-properties.html - you might want to update that.

For now, it will be sufficient, but I still think that you could go one step further and encapsulate the functionality below with some kind of shortcut property. Seems to me like the most common usage ever.

fields: {
    firstName: {
        label: 'First Name',
        placeholder: 'Type Firstname',
        rules: 'required|string|between:5,25',
        value: this.formData.firstName,
        observers: 
        [
            {
                key: 'value',
                call: ({ form, field, path, change }) => {
                    field.validate().then(({isValid}) => {
                        if (isValid) {
                            this.formData.firstName = field.value;
                        }
                    })
                }
            }
        ]
    }
}

@foxhound87
Copy link
Owner

foxhound87 commented Mar 28, 2017

Thank you for reporting that, I will update the docs soon. But still don't get what you are trying to achieve. If you are trying to derive the values to another object, just don't do that. Use the form/fields instances as much as you can and you will reduce the boilerplate.

@sebastian-zarzycki-apzumi
Copy link
Author

The point of using MobX is to work with domain model objects instead disjointed data. By not binding to model directly, you're breaking that concept.

Look: I have a User object. I want the form to edit its firstname and lastname. After the edit and submit, my User object should have the new values already populated. But currently, instead of having that, I need to get the arbitrary disjointed fields from form store, get their values and copy their values to my object back again - because they have no value (pun intened) to me in that form, if they're disconnected from my domain objects - they don't have the proper type, they don't have my domain model's methods and so on. If you have a form with 3 properties, that doesn't really matter that much, but in enterprise applications, these forms can grow very big, very quickly. Rewriting 50 values back to the object is the sheer definition of a boilerplate code.

@foxhound87
Copy link
Owner

foxhound87 commented Mar 28, 2017

A user model is not a form instance and the form instance is not a user model. A user model doesn't track all the stuff that this package is doing. What you are trying to do is connecting a model value directly to the form inputs, and this can be bad, as it will update all automatically every time user changes an input, without validating it or whatever should happen before submitting the form. Then the concept of this package is to provide a form structure for your model processing it in input and when you need to get the output to save into a model or database.

@sebastian-zarzycki-apzumi
Copy link
Author

sebastian-zarzycki-apzumi commented Mar 28, 2017

I'm not sure you've read what I wrote. My example above is showing a specific logic of updating the model when the field is valid. Of course the form model is something separate. But it doesn't mean that two-way databinding of field values back to the original domain object shouldn't be possible at the same time. I don't think there's point in arguing further. You clearly present a different mindset here. Thank you for your time and help, anyway.

@foxhound87
Copy link
Owner

foxhound87 commented Mar 28, 2017

I'm here to talk and share opinions, don't dismiss people. Connecting the form to a model would be a different approach and I don't have in my mind the whole picture and I'm interested to know more on your approach.

I wanted this package to work also without using models or for people not using mobx in their code or using it outside a mobx environment, I don't want to force them so the package can be picked by more people. Furthermore, the forms can be created by defining a simple JSON structure in input. Your first example uses mobx observables on your model, but not all users are doing that. Your second code (using validation in the observer which automatically set the value to the model) can be done in some cases, and it's ok, but how do you deal with form submissions with your approach?

Thank You

@sebastian-zarzycki-apzumi
Copy link
Author

sebastian-zarzycki-apzumi commented Mar 28, 2017

This package is called mobx-react-form. If it is not for using with MobX, then the name is misleading. If someone is using MobX, the "model" is there somewhere, eventually. Store, ViewModel, whatever, these properties have to be somewhere, is just naming.

Don't get me wrong, this library seems to be very good. I can work around my needs with some additional code. It's just missing that one core concept - 2w databinding from fields back to original model.

I don't remember submitting forms in my SPA application code for years. Why would anyone do that these days? You don't want to trigger standard request/response paradigm, with ajax and background updates, you use forms merely for taking data input and validate that input, but after that, you manipulate the data further on your own (if the need be) and pass them to your own data transport libraries, then handle the response accordingly. Form now is just a tool, not a main control mechanism in web apps.

If you need some kind of "serious" reference, take a look at ng-model in Angular apps. It creates its own form model, quite similar to yours, actually, but it syncs the form model value back to the domain object it took for databinding.

@foxhound87
Copy link
Owner

foxhound87 commented Mar 28, 2017

Anyway, I thought more and maybe this can be implemented whit some constraint. First of all, I think is bad to autosave the text fields, because the user should be sure when submit, otherwise reset functionality will be lost. Different is for inputs like booleans or arrays so I will prefer to have an option to sync only some fields. The original model will be updated only if the validation passes.

@sebastian-zarzycki-apzumi
Copy link
Author

It doesn't have to be enforced always. This behavior can be optional, so that it can be used in a different ways. Reset functionality is not lost, because you stroe all the initial values of the fields in form model anyway - so when resetting, you can set the business model back to them.

@rdiazv
Copy link

rdiazv commented Apr 21, 2017

This works for me for the model binding use case:

import { reaction } from 'mobx'

class MyForm extends Component {
  componentWillMount() {
    this.props.form.set('value', this.props.model.toJS())

    this.dispose = reaction(
      () => this.props.form.values(),
      values => this.props.model.set(values)
    )
  }

  componentWillUnmount() {
    this.dispose()
  }
}

@sebastian-zarzycki-apzumi
Copy link
Author

I'm not sure I understand the above example. Can you elaborate?

@rdiazv
Copy link

rdiazv commented Apr 21, 2017

I have a MyForm component that is bound to a model (mobx-rest) and a form (mobx-react-form):

  • When MyForm is going to mount the model assigns all its values to the form.
  • A reaction watches the form values so that whenever there is a change it sets the new form values on the model.
  • The reaction gets disposed when MyForm unmounts.

The point is that you can easily set a reaction to watch form.values() and do whatever your app needs, this way you don't have to define a per-field observer.

@sebastian-zarzycki-apzumi
Copy link
Author

Ah, I see. Yeah, it's the same approach as I've described few posts above. Yes, fortunately, there are ways to get events upon form changing and then react to these events and write back the form values to the model.

I just wanted to go a step further and make the most common case embedded directly into the framework, so that it can update the model values itself.

@akaguny
Copy link

akaguny commented Oct 24, 2019

Ah, I see. Yeah, it's the same approach as I've described few posts above. Yes, fortunately, there are ways to get events upon form changing and then react to these events and write back the form values to the model.

I just wanted to go a step further and make the most common case embedded directly into the framework, so that it can update the model values itself.

Its good idea, in my 2 projects this will be usefully!

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

No branches or pull requests

4 participants