Feature - effects

[AjaxSolutions]

I'm new to MST, so forgive me if my question has an obvious answer.

Vue has computed properties similar to MST's views, but it also has watch properties.

If you want to learn about watch properties, here's the Vue doc:
https://vuejs.org/v2/guide/computed.html#Computed-vs-Watched-Property

Also, watch this video, scroll to the 28:00 mark:
https://youtu.be/UHmFXRp0JDU?t=1691

Would it be possible to add watch properties to MST?

[mattruby]

See: https://mobx.js.org/refguide/reaction.html and https://mobx.js.org/refguide/autorun.html

…

On Tue, Jun 12, 2018 at 3:41 PM, Les Szklanny ***@***.***> wrote: I'm new to MST, so forgive me if my question has an obvious answer. Vue has computed properties similar to MST's views, but it also has watch properties. If you want to learn about watch properties, here's the Vue doc: https://vuejs.org/v2/guide/computed.html#Computed-vs-Watched-Property Also, watch this video, scroll to the 28:00 mark: https://youtu.be/UHmFXRp0JDU?t=1691 Would it be possible to add watch properties to MST? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#867>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIrcnkC_h_PNUpYn1Sxqj72a6BWPiDFks5t8CeGgaJpZM4UlHtu> .

-- -Matt Ruby- [email protected]

[AjaxSolutions]

Thanks. I think adding reactions/watchers to MST alongside views and actions would be a good feature even though this is already available in MobX.

[mweststrate]

I think this is a pretty cool idea, because it would save some boilerplate around disposers.

[mweststrate]

Big question is when should those reactions be setup. afterAttach or afterCreate? AfterAttach is not always triggered (not for roots) or multiple times (when moving nodes, which is not too common), afterCreate however does not give access to parents yet

[jayarjo]

I was wondering about the same thing recently. I will see if I can PR anything meaningful here.

[jayarjo]

I'm afraid I will need a hint on why something like this might not be a good idea.

[jayarjo]

It is not clear how to behave if the property to watch is observable array or map. What exactly should the listener receive? newValue and oldValue won't be any much meaningful in such context.

[jayarjo]

I checked what Vue does in such cases and I think they still supply newValue and oldValue despite of them being useless, as they reference the same object.

Note: when mutating (rather than replacing) an Object or an Array, the old value will be the same as new value because they reference the same Object/Array. Vue doesn’t keep a copy of the pre-mutate value.

Although not sure if we can do better and keep the immediate simplicity at the same time. @AjaxSolutions what would you expect for example?

[jayarjo]

Maybe it has sense to provide full change object instead of newValue/oldValue pair. Then users could use spread operators to extract only required properties.

[jayarjo]

Until someone suggests a better solution I thought it could simply give the full change object to the listener, unless listener explicitly requests two or more arguments (correspondingly newValue, oldValue and an optional changeType).

@mweststrate I guess your comment on 1dfe7f4 branch is still very appreciated or anyones with comparable insight into the inner wirings of MST, 'cause I should have overlooked whole lot (types is surely a definite one that I missed).

[xaviergonz]

why not just offer inside self for model types a reaction/autorun method that will add the disposer to some registry that gets disposed on destroy and let the user decide when to call it? (usually inside afterattach or aftercreate)

[jayarjo]

@xaviergonz what would be the benefit comparing to the solution in the draft? Also don't we have it already through reaction/autorun imports and addDisposer method - one could invoke these from whatever place he likes.

[AjaxSolutions]

If possible, I'd like this feature to work exactly as in Vue, which IMO is easier to use than autorun or reaction.

See the options provided by Vue's watchers.
https://vuejs.org/v2/api/#watch

This should cover 80% of use cases. For more complex requirements you can still use autorun or reaction.

See this tweet by the Evan You - the Vue creator. I agree with him that Vue is easier to use than React in part because of watch properties.

https://twitter.com/youyuxi/status/736939734900047874?lang=en

[Amareis]

In mobx-react there is disposeOnUnmount decorator now, which handles similar issue. But in react's observer components there is no problem with moment when start watching - at component creation you already have the mst tree (or mobx store).

How @mweststrate says, mst node cannot just start watching (I prefer "reacting" word, it closer to mobx universe) at contruction or at afterCreate/afterAttach - because, in general, it can reacts to whole tree, includes parents and neighbors, so node should wait for full ready tree, and, currently, there is simply no way to know about (because it depends on user intentions).

So, what we can do with it? In mobx there is three main concepts: observables, computed and, in end (or in begin?.., huh interesting question), reactions. First two also exists in mst, but for reactions there is no any analogs - we just uses plain mobx reactions, as recommended in docs.

Wait! If mst is living tree, as it described in docs, why it cannot reacts to anything? It's more like a potted tree which dies quickly without the gardener! ;)

I think, lack of side effects tools (I prefer "main effects" word, because, usually, all interesting things are happens here) is big gap for state management library. If we look at redux, there is redux-saga - side effects library. In our main application, which now in migration from redux to mst, we heavily use sagas for big part of business logic, because pure and synchronous way of redux simply doesn't fit the real world.

Of course, bunch of autoruns and reactions in some file can handle all of these effects, but why we cannot embed it into mst? There is pretty straigtforward way to do it:

1. Add effects chainable method, just as actions or views. Effect is function which subscribes to something and return itself disposer - right, just as autorun, but also it can be just some addEventListener/removeEventListener pairs or something else. Of course, effects should use actions for tree changing.

types.model('Todo', {
  id: types.identifier,
  title: types.string,
  finished: false,
}).actions(self => ({
  update: (newTodo) => Object.assign(self, newTodo),
})).effects(self => ({
  notifyBackend: () => autorun(() => api.setTodoFinished(self.id, self.finished)),
  listenBackend: () => {
    api.listenTodoChanges(self.id, newTodo => self.update(newTodo))
    return () => api.stopListenTodoChanges(self.id) //just an example, there is can be some removeEventListener or anything disposer-like
}))

2. Add affect(node) function in mst core, which recursively run all effects in tree and marks node as affected recursively.
3. Add unaffect(node) function in mst core, which dispose all effects in tree and remove affected mark recursively.
4. Add isAffected(node) function in mst core (you guessed, it returns true if node is affected!).
5. Internally, we always unaffect node before destroying.
6. Node is not unaffected on detach - user can do it himself if needed.
7. Attaching not affected node to affected parent is special case (when user make something like self.todos.push({title: 'New todo'}), if todos is affected, new todo will constructed and it's not affected yet, but it should be affected after attach in most use cases). In such case node will be affected and if user wants to avoid this, he should explicit invokes unaffect(self) in afterAttach hook (effects will not even started in that case).

I think this concept is simple enough for understanding, implementing and maintaining. What are you thinks?
@mweststrate @AjaxSolutions @jayarjo @xaviergonz

[Amareis]

Also, affect and unaffect can use optional second argument, which disables recursion. For example, on node destroying there is no mean to recursive unaffect - child nodes just will invoke unaffect at own destroying.

[luisherranz]

I also think start() and stop() are more clear. effect.isRunning feels like a question instead of an action.

[xaviergonz]

which probably could just be named "running" but I'm ok with either :)

[k-g-a]

Do I understand it right that for user this would look like:

types.model({
    data: TheOtherModel,
    version: types.number
  })
  .effects(self => ({
    // this is called every time the version field is changed after this effect has been "started" by either "immediate" or manual ```.start()```
    synchronize() { MySynchronizer.sync(self.version) } 
  }))
  .actions(self => ({
    beforeDetach() {
      // this will turn off the effect, so for whatever reason version is changed on detached node
      self.synchronize.stop()
    },
    afterAttach() {
      // turn effect on after the node is reattached
      if (!self.synchronize.isRunning) {
        self.synchronize.start()        
      }
    }
  }))

Questions:

1. will effect body be automatically wrapped in autorun by MST (as above) or should user do it himself?
2. should self.synchronize.start() execute the effect or just mark it for execution on next observed data change?

I agree that isRunning should be readonly computed.
People usually want to use library/framework to write less code, plus declarative code is considered more readable, so I do think that startImmediately should default to true. Because if one declares an effect it should just work out of the box, without needing to know that you must start it somewhere or pass some magic option.
Same about effect receiving params through start() - this adds little convenience:

// you can write
someInstance.someEffect.start(foo, bar)
// instead of
someInstance.setFooAndBar(foo, bar)
someInstance.someEffect.start()

But one should guess that adding those parameters will force his effect to be manual.

I also think that enable()/disable()/isEnabled are more semantically correct. start()/stop()/isRunning sound like this effect is async and is being executed at the moment. I mean the body of the effect, like it was a flow() and it's in the middle of the yield.

[xaviergonz]

will effect body be automatically wrapped in autorun by MST (as above) or should user do it himself?

I think

  .effects(self => ({
    synchronize: () => autorun(() => { MySynchronizer.sync(self.version) }) 
  }))

should self.synchronize.start() execute the effect or just mark it for execution on next observed data change?

I think that's up to what you use (autorun/reaction/...) / the options that are passed (fireImmediately, etc), basicaly it is just executing the function and using self.addDisposer over its return data

People usually want to use library/framework to write less code, plus declarative code is considered more readable, so I do think that startImmediately should default to true. Because if one declares an effect it should just work out of the box, without needing to know that you must start it somewhere or pass some magic option.

Agreed

I also think that enable()/disable()/isEnabled are more semantically correct. start()/stop()/isRunning sound like this effect is async and is being executed at the moment. I mean the body of the effect, like it was a flow() and it's in the middle of the yield.

Agreed

Same about effect receiving params through start() - this adds little convenience:

Sorry, I'm not sure what you mean. so you think fxs with params add value or that they don't add value?

Now I guess the question is, when should startImmediately effects start, after afterCreate?

[k-g-a]

Sorry, I'm not sure what you mean. so you think fxs with params add value or that they don't add value?

I think we should not allow params for effects as this will impose the following rule: "if you add parameters to your effect it won't be started automatically (or will throw)" - it seems counterintuitive to change behaviour based on declared function arguments.
Moreover:

function myEffectHandler(...params) {
  console.log(...params)
}
console.log(myEffectHandler.length) // it's 0
myEffectHandler('foo', 'bar') // prints 'foo' 'bar'

I do not think we should teach peope that rest syntax is transpiled to arguments parsing internally ))

Now I guess the question is, when should startImmediately effects start, after afterCreate?

At quick glance it seems that finalizeCreation is a good place to start:

finalizeCreation() {
        // goal: afterCreate hooks runs depth-first. After attach runs parent first, so on afterAttach the parent has completed already
        if (this.state === NodeLifeCycle.CREATED) {
            if (this.parent) {
                if (this.parent.state !== NodeLifeCycle.FINALIZED) {
                    // parent not ready yet, postpone
                    return
                }
                this.fireHook("afterAttach")
            }
            this.state = NodeLifeCycle.FINALIZED
            for (let child of this.getChildren()) {
                if (child instanceof ObjectNode) child.finalizeCreation()
            }
           // everything is set up, can fire effects
        }
    }

But that's not for sure )

And once again, if we really want to bring ease of use, shouldn't we reduce boilerplate as much as possible?

types.model({
  data: TheOtherModel, // {name: 'Stuff', description: '...' price: 200, discount: 0.15}
  version: types.string
})
// i'd even call those 'autoruns' as it will become autorun internally
.effects(self => ({
    // this is called every time the version field is changed after this effect has been "started" by either "immediate" or manual ```.start()```
    synchronize() { MySynchronizer.sync(self.version) } 
  }))
.reactions(self => ({
  // this changes version every time data's name or description is changed, but ignores price/discount
  updateVersion: [
    () => ({name: self.name, description: self.descriptipn}),
    (changed) => {self.version = hash(changed.name, changed.description)},
    // those could be skipped completely, but give flexibility for ones in need
    {
      fireImmediately: false,
      equals: (prev, next) => {
        // consider editing as a change only if more than 2 chars of name or 5% of description have changed
        return getStringsDiff(prev.name, next.name).lenght> 2) || getStringsDiff(prev.description, next.description).percentage > 0.05)
      }
    }
  ]
})

But if we really can not agree upon fine grained API, I do not object implementing at least effects proposed in this comment.

[Amareis]

And once again, if we really want to bring ease of use, shouldn't we reduce boilerplate as much as possible?

Effects also can react to enviroment and changes node by actions. Reactions, autoruns etc should be just sugar on top of general effects.

I think we should not allow params for effects as this will impose the following rule: "if you add parameters to your effect it won't be started automatically (or will throw)" - it seems counterintuitive to change behaviour based on declared function arguments.

If you passes arguments to effect, it obviously cannot be run automatically, it's simple contract. We can check function length right on model creation, so user sees the error instantly. And you need this check even if forbid params totally, just it will be more strict.

Now I guess the question is, when should startImmediately effects start, after afterCreate?

No, it needs to run right in create function, because of lazy.

[Amareis]

Okay, let isRunning to be just a computed readonly boolean, comment updated again.

[luisherranz]

Is there any case where effects aren't going to include either autorun or reaction, or where you want to do some computation before starting them?

Because if that's not the case, I'd vote for the less-boilerplate version of .autoruns(self => ... and .reactions(self => ....

[Amareis]

In my current app there is a lot of api subscriptions and some dom event listeners, which also can be an effects.

[xaviergonz]

I'm also against boilerplate, but I gotta say that the more general version would always be up to date with whatever feature mobx throws next + custom ones without any need to implement it in MST...

Also implementing general effects + autoruns + reactions is not really exclusive, so they could be added later if needed (plus implementing autoruns/reactions on top of effects should be trivial)

[luisherranz]

Ok then 😊

[jayarjo]

Ehm... since this went nowhere. Lets maybe revive the original idea and this draft: 1dfe7f4?

[cbovis]

We've been using volatile state to achieve this and it's been working well for us. Any downsides?

Our usage is structured like so:

// ...
.actions(self => {
  const root = getRoot(self);

  autorun(() => {
    self.someAction(root.otherStore.interestingValue);
  });

  return {
    someAction(val) {
      // do something with val to modify self
    }
  }
});
// ...

[mattruby]

I've done similar things, but I'll do them in `afterAttach` and then cleanup in `beforeDestroy` https://mobx-state-tree.js.org/overview/hooks. It's worked well for us.

…

On Tue, Jun 16, 2020 at 9:56 AM Craig Bovis ***@***.***> wrote: We've been using volatile state to achieve this and it's been working well for us. Any downsides? Our usage is structured like so: // ....actions(self => { const root = getRoot(self); autorun(() => { self.someAction(root.otherStore.interestingValue); }); return { someAction(val) { // do something with val to modify self } }});// ... — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#867 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABCW4QYAF2GSPPKPGBN4HTRW6BZBANCNFSM4FEUPNXA> .

-- -Matt Ruby- [email protected]

[coolsoftwaretyler]

Hey folks,

I'm going through the issue tracker as I do from time to time, and this is currently the "Most commented" issue, but the discussion from years ago pretty much fizzled, and I'm in agreement with the prior two comments.

I write quite a lot of "effects" myself these days, and I almost exclusively use MobX utilities like reaction inside afterAttach and clean up with beforeDestroy (I store disposers as volatile state).

I think this is a great discussion overall, and my formal recommendation for anyone who wants this is to use that approach. It keeps the MST API the same (which is already somewhat bloated), and it helps to get you familiar with some of the underlying concepts from MobX that we're using.

I understand the desire to avoid "boilerplate", but I don't think this rises to the level that I'm concerned about.

I'm going to convert this issue to a discussion for posterity and easier discoverability (I find closed issues tend to be harder to find). Thanks for all your great ideas.

If anyone out there gets a notification for this, we're still around and doin' stuff! Come hang out and contribute!