foxhound87 / mobx-react-form

Reactive MobX Form State Management
https://foxhound87.github.io/mobx-react-form
MIT License
1.09k stars 129 forks source link

Calling toJS on "extra" field property in Field strips functions #582

Closed benjamingeorge closed 1 year ago

benjamingeorge commented 3 years ago

My current code uses version 1.33.0. I have a strategy for select fields where I stored a options mapping function for select fields to be able to pull option data for selects inputs from a data provider object (returned from an api). I then created a new "ObserverSelectField" that extends from Field which added a "getOptions" method which pulled in the correct data for that particular field's key. This worked perfectly for my needs.

Updating to 3.1.0 I noticed my mapping function would get stripped off the "extra" field object. I believe this happens because of toJS called on this.$extra in src/Field.js

 @computed get extra() {
    return toJS(this.$extra);
  }

In mobx, if an object has a method that is not marked observable, and toJS is called on it, it strips the method.

My current code where "mapOptions" is preserved is simply

const extra = {
   vendorId: {
        optionsKey: "vendors",
        mapOptions: x => ({ label: x.name, value: x.id })
    }
}

But after upgrading this library, I now I have to make the "extra" props observable to preserve the method.

const extra = {
  vendorId: makeObservable(
        {
            optionsKey: "vendors",
            mapOptions: x => ({ label: x.name, value: x.id })
        },
        { mapOptions: observable }
    )
}

While it doesn't seem bad in the example, I have large form schemas with many extra fields like this. Maybe the mobx-react-form library has to be like this now, but I was just wondering if this was an oversight.

I guess my question is why would the "extra" prop on a field ever be an observable to begin with? Doesn't seem like one would ever need reactivity on his property.

foxhound87 commented 1 year ago

I removed toJS from the getter