foxhound87 / mobx-react-form

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

mobx-react-form is not compatible with MobX 6.1 #570

Closed vkrol closed 2 years ago

vkrol commented 3 years ago

mobx-react-form is not compatible with MobX 6.1. The @action decorator does not apply to some methods.

Example:

import { autorun } from "mobx";
import MobxReactForm from "mobx-react-form";

const form = new MobxReactForm({
  fields: {
    note: {
      value: "Foo"
    }
  }
});

autorun(() => {
  console.log(form.$("note").value);
});

form.$("note").set("Bar");

Console output:

Foo 
[MobX] Since strict-mode is enabled, changing (observed) observable values without using an action is not allowed. Tried to modify: Field@5.$value 
Bar 

CodeSandbox: https://codesandbox.io/s/angry-ganguly-t4f4i.

The problem is the use of mixins https://github.com/foxhound87/mobx-react-form/blob/master/src/index.js#L5-L30. I created a playground in which you can experiment with this without diving into the implementation details of the mobx-react-form https://codesandbox.io/s/mobx-react-form-mixins-playground-sbt0c.

foxhound87 commented 3 years ago

It seems just the warning, because the value is changing

vkrol commented 3 years ago

The values is changing but:

  1. This can be very noisy on large projects because messages are duplicated.
  2. These updates are performed without transactions, which can cause problems.

    They are run inside transactions. No observers will be updated until the outer-most action has finished, guaranteeing that intermediate or incomplete values produced during an action are not visible to the rest of the application until the action has completed.

https://mobx.js.org/actions.html

rafaeldsousa commented 3 years ago

@foxhound87 @vkrol any updates on this one ? I'm also seeing this happening on my app.

I know we can do,

configure({ enforceActions: "never" }); 

to suppress the warnings, but would be good to have this resolved in this module.

vkrol commented 3 years ago

@rafaeldsousa I'm waiting for feedback from @foxhound87 :)

smiley-uriux commented 3 years ago

Not trying to beat a dead horse, but I hate to see what is otherwise a really well thought out library lose adoption. Asking developers to ignore or disable warnings that we rely on to prevent elusive bugs is really not an option and impossible to sell to my team. Either way, I appreciate all the hard work that went into this library!

rdgco commented 3 years ago

@smiley-uriux - please see my latest PR. I'm with you, this mobx ecosystem is really important. I've also been updating mobx-router.

rafaeldsousa commented 3 years ago

@smiley-uriux @rdgco I think the path here is for one of the many of us to publish the module under another repo, giving the necessary credit, and maintain that there. It's clear at this point that this repo won't be maintained any longer. I'm maintaining our own forked repo as mobx-persist-store doesn't quite do all that this one does, but I'm only using it for specific apps that require object class mapping and using mobx-persist-store for anything that doesn't require that.