Mobx: Question/Discuss: Transaction nesting

[urugator]

Instead of waiting for the outermost tx, shouldn't related reactions run after the current tx ends?

The problem with current implementation is that nested transaction doesn't behave like a transaction - it doesn't ensure the state consistency after it finishes.
Any derived state is not updated. That makes code composition basically impossible when using computeds or reactions.

Here is a really simple example with computed:

class CardDeck {    
  @observable deck = [];  
  // Returns last card of deck        
  @computed get last() {
    return this.deck[this.deck.length - 1];                                         
  }
  // Draws the last card from deck     
  @action draw() {
    return this.deck.pop();   
  }
  // Draws cards until it encounters given card (doesn't work)  EDIT: actually it does..:)      
  @action drawUntil(card) {
    while(this.last !== card) {
      this.draw(); // won't update this.last, even though draw runs in transaction, which should ensure state consistency
    }
  }   
}

Notice the example is simple because everything is inside a single class, but consider something more like this:

@action action() {
  if (this.nested.value) {  // is "value" computed? does some outer TX already called some action on this.nested? does computed value depends on some object already updated in outer TX?
    this.nested2.action1()   
    this.nested2.action2() // is this.nested2 still consitent? what if action2 uses some computeds changed in action1?
    this.nested3.action() // what if this.nested3 observes this.nested2 ? 
  }
}

I think that Mobx wrongly assumes that TX is in full control of all observables involved, but that's true only for quite simple domain objects. Ensuring consistency in a bit more complicated dependency graphs is impossible when using derived values or reactions.

What are your thoughts? What are the design decisions behind current impl? Is such transaction nesting even doable?

[spion]

Off the top of my head, one possibility would be for computed values to change without triggering change events for their dependants before the transaction completes...

edit: this does appear to work when I try it here... it doesn't cause an endless loop. Seems like MobX behaves as I described already... https://jsfiddle.net/Lcqt9c54/1/

Maybe I'm misunderstanding the issue?

[urugator]

The example indeed works :) I got to the wrong conclusion because of totally different problem in my codebase.
EDIT:

one possibility would be for computed values to change without triggering change events for their dependants before the transaction completes...

I don't think this is the case... consider the length property of a deck array being a computed. The last computed is a dependant of deck.length computed, so it shoudn't be updated until the TX ends, but it is...

However the problem is still present, whenever you use autorun/reaction: https://jsfiddle.net/ozcc6d5k/3/

[spion]

@urugator the behaviour you're observing is exactly what I'd expect: computed values are updated, but reactions (autorun) don't fire until we're outside of transaction.

Do you mean that you'd like the subset of reactions that were activated within a nested transaction to run right after that transaction? I can't imagine an example like that which cannot be re-written using a computed, and computed values appear to be always consistent when accessed within a transaction...

[urugator]

Do you mean that you'd like the subset of reactions that were activated within a nested transaction to run

Exactly.

I can't imagine an example like that which cannot be re-written using a computed

Well you can't use computed, when the state is derived indirectly, that is - not derived from direct dependency, but derived through an action (that is any time you want to update a state you don't own):

// Somewhere
when(() => this.window.opened, this.content.update("Window opened")) // content intentionally knows nothing about the window

// Somewhere else
@action runApp() {
  this.window.open(); // invoking action (TX) on window
  console.log(this.content.text); // content was not updated, because this.content was updated through reaction, how should I know? It would work if content.text would be computed from window state directly
} 

Basically it's any time where you would use events or double dispatch to relax the dependency and I would say that's exactly the use case for reaction.
But currently, if reaction/autorun contains any side effects, you can't never be sure that your domain is consistent during the invocation even thought the "partical updates" are done exclusively through transactions.

[spion]

I see. The only solution I can think of is adding processReactions() to MobX. Of course it would still be impossible for MobX to know which reactions to run; it may run reactions that aren't related to the transaction at all (like showing the window) resulting with glitchy behaviour.

Pretty sure that the alternative i.e. partially "committing" nested actions would be a serious breaking change...

Unless its explicitly requested:

E.g. the API would be runWithReactions(() => this.window.open()) which would be sort of complementary to runInAction and only valid within actions

p.s. Afaik the point of reactions isn't to mutate MobX state, but to trigger external (as in, external to MobX) side effects? I'm not sure if the cost is worth the benefit when using them this way. Cycles are extremely easy to create.

[urugator]

The only solution I can think of is adding processReactions() to MobX.

Such method would probably have to accept some TX ID. However I think it should be done automatically at the end of the each (even nested) TX.
I think that TX tracking shouldn't be a problem because of synchronous nature of MobX. Even currently the accessed observables must be somehow tracked and associated with current TX, so that their observers can be invoked at the end.
In theory, instead of single TX, we just need a stack of TXs and associate accessed observables with the TX on the top of the stack (the current one):
-> entering TX: txStack.push(new TX)
-> observable is modified: txStack.top().add(getReactionsForObservable(modifiedObservable))
-> leaving a TX: txStack.pop().getReactions().invoke(); //continue user code
I don't know the technical background so this is just my imagination...

Afaik the point of reactions isn't to mutate MobX state, but to trigger external (as in, external to MobX) side effects?

It might be the case. I had an impression that reactions are quite often offered as a possible solution for a synchronization of two observables (for example when computation would be too expensive)...
Documentation isn't particulary specific about what the side effect should or shoudn't contain either.
Applying the reactions "progresivelly" after each atomic state mutation in order to ensure consistency at any point of execution somehow seems logical.
On the other hand I realize it would cause a lot more (mostly unneccessary) re-renders of react components (or observer updates in general).
So yea, maybe I am actually looking for something else, something like observe which respects individual transactions...? Or a way to define the "system border" where reactions can be batched?

Cycles are extremely easy to create

The cycles are harder to spot, but not easier to create. Well yes, there might be an implication between the two, but my point is that existence of the cycle is not a result of used solution.
The cycle is a result of cyclic dependencies (usually transitive), but it doesn't matter if the dependency is expressed by interface, event, constructor parameter or reaction. The dependency cycle is an attribute of a model, it won't be removed by switching from event subscription to passing constructor parameter...
Each solution has it's price, the explicit dependencies are easy to understand, but the tight coupling might be undesirable.

[mweststrate]

Good questions. After lot's of experimenting I'm pretty sure the current semantics are pretty optimal to in at least 95% of the cases

It is indeed correct that reactions are only running if the outer transaction completes.

However, if you inspect a computed value before a transaction ends, the computation of that value (and all values it depends) is pulled forward, so computed values will always be consistent with the state.

Actions only complete when the outermost one is complete. Firing immediately affected reactions immediately would large remove the point of transactions, take the following (simplified, but not contrived!) example:

@observer class Component {
   @observable name
   @observable age
   render() {
      return this.name + this.age
   }
   @action setName(x) { this.name = x }
   @action setAge(x) { this.age = x }
   @action setNameAndAge(x, y) {
      this.setName(x)
      this.setAge(y)
   }
}

Having transaction run after each action would kill the compositional properties of actions; action setNameAndAge would render the component twice (roughly the equivalent of not using action at all!), while with the current semantics it will render only once.

Reactions are indeed _not_ intended to keep observables synchronized with each other, but mainly to manage side effects like networking, logging, drawing etc. In rare cases computed might not suffice to keep data synchronized, but I would always start with computed until you know for sure it won't suffice. An example where it won't suffice is for example a two variables that are bidirectionally bound and writable. However in these cases you are better of with observe, which don't use transaction semantics and always fire synchronously on each write.

[urugator]

@mweststrate Thanks for clarification.

Firing immediately affected reactions immediately would large remove the point of transactions

I think the main point of TX is ensuring consistency. Batching the changes across TXs, while useful in certain context, is not an inherent property of TX itself and from some perspective actually defeats the prupose of TX, because it doesn't ensure consistency.
I understand the idea, I just kind of disagree with the statement about the point of TX.
I wonder if there is a sane way to find out, which reactions can be batched and which would have to run immediately...

Having settled how and why it works, do you think it would worth having something like reaction, which could be used as a replacement for events?
Normal events are processed synchronously (immediately when fired), so as suggested, the side effect (listener) would have to run as soon as possible, that is after the current TX ends (to ensure state consistency inside side effect)
Why:
1) ability to reuse existing means - why should I introduce onOpened event when there is already an observable field opened
2) like in reaction, the subscription part is dynamic (it is a function) - you subscribe for data you're interested about, not for artifical event

[mweststrate]

@urugator I think it is not impossible to add something you need, but I think it would be highly confusing all those subtle semantic differences. I feel that I'm already stretching in with the difference between transaction based concepts like autorun, and non transaction respecting low level methods like observe.

I think you can address most of your specific problems by splitting more things up into computed and reactions. So any value that you need in your code should be computed, even though you will need an actual side effect in the end. So if somehow the current popup text is a value require somewhere else, make it computed, and only consider opening / rendering that text into a popup a reaction. The common denominator for side effects in MobX is that they preferably not something that can be 'seen' from the code; like logging the console, making network requests, or updating the DOM. These are all things that interop with the 'outside' world from the perspective of your app code (even though you technically can query the DOM, I think the other side effects cannot even be detected from inside JS).

[urugator]

So if somehow the current popup text is a value require somewhere else, make it computed, and only consider opening / rendering that text into a popup a reaction.

Yes, this works if reaction is defined on "the topmost layer" or "at the system border", where all possible state mutations start and end.
But if you want to introduce another layer and use simple composition, reusing that piece of code is not possible, because when you invoke action on that reused code, that new layer won't see applied state mutations after the (inner) action ends.
Used reaction probably had some reason, so you can't just magically get rid of it. What you have to do is to push the reaction to that new topmost layer. But doing so will remove the whole point of reusing that object, because the point of that object was probably to provide a bridge between two (loosely coupled) objects by using reaction and encapsulate this logic into reusable component...

If I can't in fact use reaction (or suggested alternative) inside the system (only at it's border), then only tool remaining is computed (in case I want to use MobX for state management).

Consider synchronizing opened status of 2 windows:

One way to do that with computeds is to share some observable object like openedStateHolder, which has to be passed to windows upon creation.
First problem is, the openedStateHolder observable have to exist, before the windows are created. Compare this to reaction/eventListener, which can be created/registered ad-hoc.
Secondly how do you break the synchronization? Again compare this to reaction/eventListener, which can be disposed/removed ad-hoc.
How do you easily introduce third window, which has inverted behavior?
The relationship here is wrong - the "synchronizer" should depend on window, not the other way around.
The opened state shouldn't be a dependency, it should be owned by window - the window object was introduced as an abstraction of openeable state, passing the openedStateHolder defies the purpose of window object.

Another way would be to pass first window into the second (so the state of second is computed from first), but it shares some problems and doesn't provide required abstraction.

Here is the deal: no matter which workaround for reaction I use, the MobX is actually NOT used for state management IN ANY WAY... If I would remove the @computed from getters, the state of all objects would be still consitent at any point of time.
By replacing reaction, I actually remove the need for MobX altogether (from the perspective of system alone)... the MobX is only used to synchronize system with 'outside' world, but it has no use inside the system itself...
That just feels somehow limiting, considering everything needed is already there... or maybe there are still some misconceptions?

[andykog]

@urugator, what you described can be nicely solved by using computed: https://jsfiddle.net/Sl1v3r/1vfrgj5p/
So are there some real needs to use reactions for managing state instead of running side effects?
Its sad, that reactions were promoted to be used like that.

disputable proposal

I've been thinking about some mechanism of assigning computed value to observable value, that would even redeem the need to predefine placeholder for transformer ($isOpenTransformer) (would not be easy to implement in current codebase though):

var value = observable(0);
value.set(computed(() => 1, /* setter is restricted here */));
value.get() // should be 1
value.set(2);
value.get(); // should be 2

[andykog]

@urugator, red your comment again and realized that you were saying about bi-directional syncing, and you are right: there have to be some external state holder if you are using computeds. That makes sense as something have to be the source of truth according to MobX concept, and this allows you to keep your state as pure computations.

[urugator]

what you described can be nicely solved by using computed:

Nope. Remove all the @computed, @observable, @action and it will still work. You haven't used anything from MobX to solve the problem at all.
It works exactly the same as openedStateHolder, you've just used callback as a state holder and provided default state holder so it doesn't have to be passed during window creation, but all mentioned problems remains.
Notice:

@observable $isOpenTransformer = () => false;
// <=>
@observable $openedHolder = { opened: false };

@computed get isOpen() {
  return this.$isOpenTransformer();
}
// <=>
@computed get isOpen() {
  return this.$openedHolder.opened;
}  

myWindow.bindIsOpen(() => win1.isOpen);
// <=>
myWindow.setOpenedHolder(win1.$openedHolder);

[andykog]

@urugator, my idea was

1. to store a transformer function computing value instead of storing the actual value
2. to replace the transformer instead of creating/disposing reaction.

[urugator]

to store a transformer function computing value instead of storing the actual value

Yea, it's the same thing, either transformer always yields the same value, so it in fact represents the actual value (the value is just obtained by invoking the function, instead of calling getter) or it computes the value from something, which still needs to be provided externally (just shifts the problem)...
However the main point is that such solutions don't require MobX ...the syncing of windows is not done by MobX... this claim applies to any solution which doesn't use reaction (or autorun). So yes, such synchronization problem can be solved differently, but it can't be solved by utilizing MobX without using reaction. And because you can't use reaction inside the system, it means you can't use MobX to solve this problem at all...

[andykog]

@urugator, I'm struggling for using MobX reactions internally as less as possible, I believe it leads to creating system that are hard to predict and understand. But I can see your point. If this is really indispensable, I'd better made reactions configurable like this:

reaction(() => {}, () => {}, {
  fireImmediately: true,
  fireInsideActions: true,
})

[urugator]

I believe it leads to creating system that are hard to predict and understand.

I mostly agree, events/reactions can be easily misused or overused. However they are natural tool to bridge two independent systems.
Problem is MobX doesn't allow to introduce a border inside the system itself, that is spliting the system into independent subsystems, because there is always only single transaction context.
EDIT: And again notice the paradox here: If using reactions leads to creating system that is hard to predict and understand, than it actually means that using MobX leads to a system that is hard to predict and understand, because without any observers established by reaction/autorun, the MobX isn't relevant to the system (does nothing)....

fireInsideActions: true

Iteresting idea, it should probably named differently, it's more about TX than actions.
I was also thinking about something like this:

const txContext = Mobx.createTransactionContext("id?"); // creates independent TX context, by default everything runs in global TX context 
doSomething(); // this now runs in new TX context
txContext.commit(); // or .applyMutations/Changes, .flush() ... applies state mutations as a result of changes performed inside TX
// or maybe
const txContext = Mobx.createTransactionContext("id?"); // creates independent TX context, by default everything runs in global TX context
txContext.run(doSomething);
// EDIT: probably the above is unneccessary, we could just instruct the action/tx to create own context
Mobx.transaction(worker, { createBatchingContext: true }) // ceateNestingContext, createTransactionContext...

Or maybe being able to provide TX ID to action/tx and refer to this ID in reaction options...not sure about usability...

[urugator]

I've been thinking about the difference of specifing the behavior on reaction and on transaction level.
When specified on reaction, the advantage is it doesn't affect other reactions like invoking react components (which is quite useful I think).
The disadvanatage is it doesn't allow batching mutations of subsystem, only the innermost TX is taken into account.
To supported both, one would have to associate reaction with a particular transaction, by ID or by passing transaction reference to reaction... something like this?:

const tx = Mobx.transaction(window.open);
Mobx.reaction(() => window.opened, () => window2.open, { transactionContext: tx });
// the ID would be probably easier to use because you could easily associate multiple TX, with single reaction

However batching is meaningful only when reaction results in expensive operation (like updating DOM), so the usage might be quite limited...
EDIT: I have also relalized, that such expensive operation would have to run asynchronously, which creates the system border already (async callback is not invoked as part of a running TX)

[andykog]

@urugator, note that transaction's are deprecated in favour of actions. I agree, that there seems to be very little benefit (comparing to complexity boost) from providing specific transactionContext instead of the ability to force reactions to always react immediately, even inside actions.

[spion]

@urugator

Lets say you have the following stateful components: Window, Titlebar, Tabs.

Titlebar has a title observable, and a computed text titleToRender which can be shortened with an ellipsis due to smaller window size

Lets say you wish to wire these in a reusable component WindowWithTabs that combines them all. This window needs to react to when tabs change, and set the window's non-shortened title to be originalTitle + ": " + activeTabName

What I don't understand is why you have code that relies on this reaction happening immediately. Like,

windowWithTabs.setActiveTab(4)
logger.log(windowWithTabs.title.text) // why not a reaction again?

All examples I can think of could be also implemented with reactions instead. A simple realistic example (or better yet, the actual problem you are trying to solve) would be a big step towards a solution IMO.

[urugator]

@spion What you're suggesting is to avoid ALL reads inside actions, because you can't never be sure if a value of some prop is a result of some reaction.
Is it achievable? Maybe it is, I don't know...what about:

open() { 
  if (this.opened) return; // was this.opened updated by reaction somewhere, inside outer action (caller) perhaps?
  // ...
}

[spion]

@urugator I'm actually suggesting a complete description of the original problem. Perhaps its just me, but I'm unable to piece it together from the previous comments. Therefore anything I "suggest" right now seems to be totally useless. (its based on my different example, so it doesn't address your issue)

[urugator]

Therefore anything I "suggest" right now seems to be totally useless

No it isn't, I think you might be actually right :)
Prohibiting access to all observables from anywhere outside of computed/reaction/autorun/when could be THE solution... I need some time to put my current hypothesis into use and then I will share my findings with a hopefully better explanation. For the time being, feel free to close the issue if necessary.

[mweststrate]

Closing for now. I think changing the semantics has too much impact right now and also too many confusing edge cases, although there are some with the current behavior as well.

[optimatex]

@mweststrate actions act just perfect when they are composed. It's really incredible