Mobx-state-tree: Introduce new syntax to declare actions and volatile state

Created on 1 Aug 2017  路  21Comments  路  Source: mobxjs/mobx-state-tree

This issue is basically a placeholder for @mattiamanzati' s awesome idea, as demonstrated here

Example:

const Todo = model({
    id: t_number,
    name: t_string,
    done: t_boolean,
    remoteId: t_number
}).actions(self => {
    function toggle() {
        self.done = !self.done
    }

    function setName(name: string){
        self.name = name
    }

    const saveToServer = async(self)(function*(){
        // post to server...
        self.remoteId = yield fetch('http://localhost/sample')
    })

    return { toggle, setName, saveToServer }
})
brainstorminwild idea breaking change

Most helpful comment

Thanks but you made me think about that idea @mweststrate :P
The idea is that the only required piece to construct a model will be properties. Everything else will be attached at construction time, and so stored as function that will be called on the instance at construction time.

interface IModelType<S, T> extends IType<S, T> {
    named: (newName: string) => IModelType<S, T>
    props: <P>(props: { [K in keyof P]: IType<any, P[K]> }) => IModelType<S, T & P>
    views: <V>(fn: (self: T) => V) => IModelType<S, T & V>
    actions: <A>(fn: (self: T) => A) => IModelType<S, T & A>
}
model(props: { [K in keyof P]: IType<any, P[K]> }): IModelType<S, P>

Because name is already a concept in MST types, we should name it either "named" (I like most) or "withName".

overall changes:

  • now views will be separated from props, even stricter typings for model could be used, I would continue to support the "pass-in-the-default-primitive" behaviour.
  • state will no longer exists. Each scope for actions will be executed at construction time. Feel free to stick in there your "state". It's even safier because you can't access it from outside. Great place to save pending promises, websocket connections, etc...
  • wrapped actions will be only the exported ones. Means performance improvements, because any internal action call wont trigger isRunningActions checks.
  • there is no more need to wrap view functions, because isAlive checks will be performed on properties access
  • each "self => ({})" function could be considered a "construction" scope. Basically will be called at model construction time. We'll put somewere a flag to mark this construction phase, so when calling utility functions like async(self, "fetchFromServer")(.....), the async function can ensure that it's being called at construction, and not inside an action/callback.
  • "no-more-this" to prevent issues with typings and different settings of noImplicitThis and things like async generator issues. You can use instead "self" being passed as parameter.
  • ideally no more "afterCreate" because the function scope itself is already construction phase. React does the same, constructor -> actions(self => ({})), componentWillMount -> beforeAttach, componentDidMount -> afterAttach, willUnmount -> beforeDetach.

All 21 comments

Thanks but you made me think about that idea @mweststrate :P
The idea is that the only required piece to construct a model will be properties. Everything else will be attached at construction time, and so stored as function that will be called on the instance at construction time.

interface IModelType<S, T> extends IType<S, T> {
    named: (newName: string) => IModelType<S, T>
    props: <P>(props: { [K in keyof P]: IType<any, P[K]> }) => IModelType<S, T & P>
    views: <V>(fn: (self: T) => V) => IModelType<S, T & V>
    actions: <A>(fn: (self: T) => A) => IModelType<S, T & A>
}
model(props: { [K in keyof P]: IType<any, P[K]> }): IModelType<S, P>

Because name is already a concept in MST types, we should name it either "named" (I like most) or "withName".

overall changes:

  • now views will be separated from props, even stricter typings for model could be used, I would continue to support the "pass-in-the-default-primitive" behaviour.
  • state will no longer exists. Each scope for actions will be executed at construction time. Feel free to stick in there your "state". It's even safier because you can't access it from outside. Great place to save pending promises, websocket connections, etc...
  • wrapped actions will be only the exported ones. Means performance improvements, because any internal action call wont trigger isRunningActions checks.
  • there is no more need to wrap view functions, because isAlive checks will be performed on properties access
  • each "self => ({})" function could be considered a "construction" scope. Basically will be called at model construction time. We'll put somewere a flag to mark this construction phase, so when calling utility functions like async(self, "fetchFromServer")(.....), the async function can ensure that it's being called at construction, and not inside an action/callback.
  • "no-more-this" to prevent issues with typings and different settings of noImplicitThis and things like async generator issues. You can use instead "self" being passed as parameter.
  • ideally no more "afterCreate" because the function scope itself is already construction phase. React does the same, constructor -> actions(self => ({})), componentWillMount -> beforeAttach, componentDidMount -> afterAttach, willUnmount -> beforeDetach.

@mattiamanzati Sorry to chime in, I'm not that well-versed in TS; would this be a good example in plain JS?

const Todo = model({
  named: 'Todo',
  props: {
    id: t_number,
    name: t_string,
    done: t_boolean,
    remoteId: t_number
  },
  views(self) {
    return {
      shortName(length) {
        return self.name.substring(0, length);
      }
    };
  },
  actions(self) {
    // actions can be kept private/internal by *not* exporting them
    const uncheck = () => {
      self.done = false;
    };

    return {
      toggle() {
        self.done = !self.done;
      },
      setName(name) {
        uncheck();
        self.name = name;
      },
      saveToServer: async(self)(function*() {
        // post to server...
        self.remoteId = yield fetch('http://localhost/sample');
      })
    };
  }
});

If that's correct, I would say I would prefer that over any fluent API: it seems simpler, more flexible and less problematic (same reasons to use flow/compose over chain in lodash, or date-fns over moment).

This gets rid of this completely? Would there still be computed get properties?

I'm excited, thanks to both of you!

Yes, it will get rid of this completely :)
computeds can still be defined in the views section using get computed(){ return self.todos.length } for example :)
Type-wise that syntax is a little harder, so in the end its mostly a point of view :)

@rchanou the most important difference is that the fluent interface produces a fresh type on each step (all type definitions are immutable), so the type self in actions, refers to the type produced by the previous, props, views, etc calls

P.s. also note that without local state or private methods, actions can be simply written as:

type.actions(self => ({
      toggle() {
        self.done = !self.done;
      },
      setName(name) {
        self.name = name;
      },
}))

This makes simple action blocks more easy to write :)

Guys, looks awesome (although we just finished porting our codebase to current version of MST, more work awaits ;).

Yet, related to this, will this also simplify the Typescript model definition. It's often very difficult to read through all those type unions to see the shape of the final type. I'll probably open a new ticket, but it would be great to ditch "type" and introduce "interface", to see only the final produced type (although it still may not be possible to eliminate &'s completely)

Yet, I wonder how will you type those private functions from the "actions" bit. But you're the magicians here ;)

@tomitrescak Private functions will only have "self" typed, because they are indeed private :P
We already have a codemod I wrote that did mostly all (95%) of the job of migrating the tests codebase, would be great if before releasing the new version you could make a backup copy of your codebase and try it over the codebase to eventually address unsupported edge cases :)
You can find it here https://github.com/mobxjs/mobx-state-tree/blob/cd339acf927c6844bd11e27fda4da097fed8acc8/codemod/src/index.ts (there should be also the compiled version in the same branch)

This new API seems elegant indeed.
Will it possible to add/remove actions during runtime?
I mean: will doing .actions(moreActions) multiple times affect the existing instances?

No, calling .actions will produce a new type :) @dagatsoin

Great idea, this would make code more explicit, by labeling every argument block and even allow grouping actions in chunks, possibly also split in modules!

If I'm not wrong it also allows prototypal inheritance without subclassing and immutable composition, for this reason it reminds me of stampit.

Just a couple of questions/proposals:

  • Is the interface intended to be used both with configuration object and with immutable setters?
  • Are the views a new concept?
  • Would you consider adding a hooks property for lifecycle hooks, so that they are not mixed between actions? It would be way more explicit and be less ambiguous for MST beginners, I think.

"Are the views a new concept?"
No, in the past computed gets and functions defined in the properties section were views, basically they can only read the state and not modify it! :)

"Would you consider adding a hooks property for lifecycle hooks, so that they are not mixed between actions? It would be way more explicit and be less ambiguous for MST beginners, I think."
Was one of my concerns. I am +1 for that, but it's hard and not fully pratical sometimes because you cannot share volatile state vars (scoped vars) between methods and lifecicles :)

I did'nt fully understood the first question :P

Ah, now I got what you mean by views.

About the first question, sorry for not being clear. I mean supporting both:

  • t.model({ actions: ... }) as a configuration object
  • t.model().actions(...) as an "immutable setter"

Could you explain better this part?

not fully practical sometimes because you cannot share volatile state vars (scoped vars) between methods and lifecycles

Thanks!

The only mode supported by t.model with be with properties as argument, so properties are the only configuration object available :)

So it won't be possible to do t.model(...).actions(...)?

Sorry, I explained myself wrong, I meant that it wont be possible to do t.model({ properties: {}, actions: {}}), the t.model(arg) arg can be only the properties :)

Sorry, I understood @rchanou example was right, instead the correct one would be:

const Todo = model({
  id: t_number,
  name: t_string,
  remoteId: t_number,
})
  .named('Todo')
  .props({ // Can add props separately
    done: t_boolean,
  })
  .views(self => ({
    shortName(length) {
      return self.name.substring(0, length)
    },
  }))
  .actions(self => {
    const uncheck = () => {
      self.done = false
    }

    return {
      toggle() {
        self.done = !self.done
      },
      setName(name) {
        uncheck()
        self.name = name
      },
      saveToServer: async(self)(function * () {
        self.remoteId = yield fetch('http://localhost/sample')
      }),
    }
  })

Is this right?

Exactly! :)

released with 0.10!

One of the thing I liked with the old implementation was to be able to define some properties in the volatile state, use them in my react view, but not getting them in the snapshot generated by getSnapshot. Would that be possible with the new syntax?

Please see #456 which is moving forward that direction :)

Was this page helpful?
0 / 5 - 0 ratings