xaviergonz / mobx-keystone

A MobX powered state management solution based on data trees with first class support for Typescript, support for snapshots, patches and much more
https://mobx-keystone.js.org
MIT License
555 stars 25 forks source link

instantiate model class when to assign in modelAction #127

Closed misaeldossantos closed 3 years ago

misaeldossantos commented 4 years ago

When I try to assign a js object to an attribute of the model, the object is not instantiated, so when I try to call a method, an exception is raised because it does not exist.

Ex.:


@model("todo")
class Todo extends Model({
  name: prop("")
}) {
  @modelAction
  setName(name) {
    this.name = name;
  }
}

@model("todos")
class Todos extends Model({
todos: prop(() => [])
}) {

   @modelAction
   fill() {
       this.todos = [{name: 'test 1'}, {name: 'test 2'}]
   }
}

const todos = new Todos({})
todos.fill()
todos.todos[0].setName("name changed")

it raise this error: TypeError: todos.todos[0].setName is not a function because class was not instantiate. But if I do: this.todos = [new Todo({name: 'test 1'}), new Todo({name: 'test 2'})] works.

It is too bad, because deep attrs (with array) is hard to instantiate manually, especially after calling an api...

xaviergonz commented 4 years ago
this.todos = [{name: 'test 1'}, {name: 'test 2'}]

I assume you want the todos array to be an array of Todo instances, so you'd need to do

this.todos = [new Todo({name: 'test 1'}), new Todo({name: 'test 2'})]

// or
this.todos = [{name: 'test 1'}, {name: 'test 2'}].map(data => new Todo(data))

mobx-keystone (unlike mst) does not do implicit conversions from snapshots to instances automatically in order to be more explicit and have more specific TS typings.

misaeldossantos commented 4 years ago

No plains to add this feature?

xaviergonz commented 4 years ago

In order to implement it the use of tProp then would need to be required (so it actually knows what kind of model the array is made of). You can see https://github.com/xaviergonz/mobx-keystone/issues/123 for more details