Converting function from `useLocalStore` to action?

[Maaartinus]

I also ran into #235 and I wonder, whether useLocalStore could be made more usable with enforceActions: "always". Starting with @madeofsun's sanbox, the user can try to wrap the body in runInAction, which fails because of unbound this, they can try

function initStore() {
  const result = {
    numberOfApples: 3,
    addApple() { // broken
      runInAction(function() {
        result.numberOfApples += 1;
      });
    },
  };
  return result;
}

only to find out that result is not the object they'll use later, so they can waste quite some time. I was mislead by #170 stating that runInAction is necessary, but AFAIK, with the state-creating function declared externally, only action works.

I wonder whether a function like declareActions(result.addApple, result.removeApple, ...) or alike is possible (I tried and failed, damn "this"). I mean, something similar to mobx.decorate. Or maybe, a variant of useLocalStore allowing to declare somehow the actions, so that the user wouldn't have to struggle themselves.

[danielkcz]

Yes, we have been discussing just yesterday and it might be indeed worth it to expand decorate to accept objects. I don't have specific implementation advice, but I am sure there is some way.

[mweststrate]

having a third argument to useLocalStore to accept a decorators map sounds like a neat solution!

[danielkcz]

@mweststrate I think it would make more sense to extract this to mobx and wrap in the hook in here for convenience.

[mweststrate]

Fair point, cross link, should be a use case for https://github.com/mobxjs/mobx/issues/2288

[Maaartinus]

having a third argument to useLocalStore to accept a decorators map sounds like a neat solution!

Isn't a whole map too much? Properties are observable and getters are computed and that's fine. It's just that methods may be either views or actions, so enumerating the actions (usually less common than views) should do, shouldn't it?

Obviously, that's not as flexible as decorating objects, where you can get non-observable properties and non-computed getters.... but 99% of times it's just a forgotten decorator. :laughing:

Maybe the decorator map could be used to opt-out like

decorate(Timer, {
    // start: observable, // the default can be omitted
    someNonObservable: plain,   
    // elapsedTime: computed, // the default can be omitted
    someNonComputed: plain,
    tick: action
})

[danielkcz]

I agree. The original decorate could be improved for classes as well. If you omit property, there should be a default fallback behavior. Then you can use options map only if you want something else or possibly specify options.

[urugator]

As noted many times before, we cannot make class automatically observable because we don't know which props it has - props are not on prototype, but on instance... such introspection has to be done at the end of constructor, which isn't "patchable", because the "class" and "constructor" are physically the same thing...

[johot]

I also ran into this one today. We are long time users of MobX at work and love it. Now i'm looking into using the useLocalStore and object literals instead of classes. We really like the strict mode setting not just for transactions but also for enforcing people to use common methods of changing state so all logic can be kept in a single place and people aren't changing the properties of the store directly.

From what I understand the useLocalStore will give us the transaction support automagically but we can't get strict mode working since I can't find an elegant way of decorating the methods with action.

What is the current thinking about this one month later? Disabling strict mode is ofc one way but it would be a pitty I think :(

[johot]

Ok follow up :)

To keep the code relatively clean I did this which allowed strict mode to work.

export const createTodoStore = (todos: Todo[]): TodoStore => {
  const todoStore = {
    todos: todos,
    removeTodo(todo: Todo) {
      this.todos = this.todos.filter(t => t !== todo);
    },
    toggleTodo(todo: Todo) {
      todo.completed = !todo.completed;
    }
  };

  // Make the follows methods into actions
  todoStore.removeTodo = action(todoStore.removeTodo);
  todoStore.toggleTodo = action(todoStore.toggleTodo);

  return todoStore;
};

Would this be an ok approach or does it have any problems I am missing? The decorate function didn't seem to work for non classes, but this did the trick.

[danielkcz]

@johot Yea, there isn't probably a better way right now. Hopefully, something will come from recent discussions about next MobX V6. I will keep it open here to keep track of the problem.

[urugator]

@johot just in case it's not obvious, it's the same as

removeTodo: action(function(todo: Todo) {
   this.todos = this.todos.filter(t => t !== todo);
})

Note you just cannot use arrow function

[urugator]

For the time being, maybe this could be used: https://github.com/mobxjs/mobx/issues/2325#issuecomment-619534231 It's like transaction (provides batching), but doesn't violate strict mode unless it contains mutations and you call it from render (or other derivation). A compromise between safety and conviniency.

[danielkcz]

FYI this will get better with upcoming MobX6 and all methods will be true actions by default.

[mweststrate]

This will be addressed by #302, closing for now.