is @action really necessary?

[marialovesbeans]

In the documentation, it says the advantage of@action is batching multiple set, so affected component is only re-rendered once.

So I went on and did a test. I omit @action and strict mode, everything still works as intended. The affected component also gets re-rendered once, not multiple times as stated in the documentation.

So I'm wondering if the 2 code block below actually achieve the same thing? If so, can I simply omit @action and runInAction for the sake of less code? Thanks!

With @action, runInAction(), and strict mode

configure({ enforceActions: true });

  @action
  addItem(item) {
    this.status = "pending...";
    this.status = "pending2...";
    this.status = "pending3...";
    setTimeout(() => {
      runInAction(() => {
        this.status = "done";
      });
    }, 1000);
  }

Without @action = less code, and still works the same, component also gets re-rendered once (for the first 3 set operations)

  addItem(item) {
    this.status = "pending...";
    this.status = "pending2...";
    this.status = "pending3...";
    setTimeout(() => {
        this.status = "done";
    }, 1000);
  }

So what's the difference here really?

[urugator]

Have you called addItem from react's event listener like onClick ? If that's the case the react batched three mobx's forceUpdate calls. Try to test it against autorun or reaction ...

[marialovesbeans]

so if the action triggerer is an react event, the performance difference is negligible, right?

[urugator]

If the code runs synchronously inside react's event handler and there are no additional reactions/autoruns, then most likely yes, the performance difference is arguably negligible. Note that anything else won't be batched and will cause re-render on every state mutation - defered code (.then/setTimeout/...), native event handlers (window.addEventListener), etc

[marialovesbeans]

I see, so in Redux, when multiple dispatch() is called in a defered code, they seem to cause multiple renders by default. Is there anything similar to runInAction to batch them? Or I have to implement shouldComponentUpdate.

[urugator]

In react you can batch multiple setStates/forceUpdate with ReactDOM.unstable_batchedUpdates. As mentioned event handlers use this automatically.

In redux the dispatch also calls ReactDOM.unstable_batchedUpdates, to avoid multiple renders when multiple different components are notified. Afaik you can't batch multiple dispatch calls with redux itself, but you can obviously wrap them into another ReactDOM.unstable_batchedUpdates. However you can batch multiple redux actions by wrapping them into another "BATCH" action and dispatching it instead, like this.

Mobx also uses ReactDOM.unstable_batchedUpdates for the same reason. However mobx is not limited to component updates, it has it's own generic concept of side effects, known as reactions, therefore it has own batching mechanism, which makes sure that all (affected) reactions are invoked right after the action/transaction finishes and no more than once.

[marialovesbeans]

Awesome, got it! Thanks a lot!

[roeycohen]

still about this topic, does it makes sense to use action decoration on a method that just makes one modification?

@action
setPending() {
    this.status = "pending...";
}

[urugator]

It makes sense mainly from consistency perspective. Performance wise there is no benefit. The action additionally wraps the function with untracked to prevent unintended subscriptions, when executed inside tracking context (not very common). It can also help with debugging as it reports the invocation on devel env. Check the impl for details.

[devuxer]

@urugator

You wrote...

If the code runs synchronously inside react's event handler and there are no additional reactions/autoruns, then most likely yes, the performance difference is arguably negligible.

So, let's say I have this handler for a react event...

function handleUpdateLocation(lat, long) {
    viewModel.lat = lat;
    viewModel.lon = long;
    // assume lat and lon are observables
}

Are you saying I would see no gain in performance by wrapping this function with action? (Assume no autoruns or reactions are involved, only computed's.)

What about in this case?

async function handleUpdateLocation(lat, long) {
    viewModel.lat = lat;
    viewModel.lon = long;
    success = await store.update(viewModel):
    if (success) viewModel.isOpen = false;
}

Here, lat and lon are set synchronously but isOpen is set some time later. From a performance standpoint, how would this be any different than if I wrapped handleUpdateLocation with action and if (success) viewModel.isOpen = false; with a runInAction?

What if multiple observables were mutated after the await, like so?

async function handleUpdateLocation(lat, long) {
    viewModel.lat = lat;
    viewModel.lon = long;
    success = await store.update(viewModel);
    if (success) {
        viewModel.status = "OK";
        viewModel.isOpen = false;
    }
}

Is this where I would finally be hurt by not using action/runInAction?

My guess is that there wouldn't really be much performance benefit of wrapping handleUpdateLocation in action but there would be for wrapping the code after the await in runInAction, as shown below. Is that correct?

async function handleUpdateLocation(lat, long) {
    viewModel.lat = lat;
    viewModel.lon = long;
    success = await store.update(viewModel):
    if (success) runInAction(() => {
        viewModel.status = "OK";
        viewModel.isOpen = false;
    })
}

[urugator]

@devuxer

if I wrapped handleUpdateLocation with action

The action has no effect, because the first line of async function is already deferred (runs outside of action), so you have to use runInAction here as well or mobx's flow. Simply put async functions are unbatchable by both mobx/react (can change in the future).

What if multiple observables were mutated after the await, like so?

Each mutation outside of action results in individual forceUpdate() call. Considering there is a single component observing all the observables:

// No batching - 4 update requests - 4 renders
async function handleUpdateLocation(lat, long) {
    viewModel.lat = lat; // forceUpdate() -> render()
    viewModel.lon = long; // forceUpdate() -> render()
    success = await store.update(viewModel)    
    viewModel.status = "OK"; // forceUpdate() -> render()
    viewModel.isOpen = false; // forceUpdate() -> render()   
}

// Mobx level batching - 2 update requests - 2 renders
async function handleUpdateLocation(lat, long) {
    runInAction(() => {
      viewModel.lat = lat;
      viewModel.lon = long;
    })  // forceUpdate() -> render()  
    success = await store.update(viewModel)    
    runInAction(() => {
      if (success) {
        viewModel.status = "OK";
        viewModel.isOpen = false;
      }      
    }) // forceUpdate() -> render()       
}

// React level batching - 4 update requests - 2 renders
async function handleUpdateLocation(lat, long) {
    ReactDOM.unstable_batchedUpdates(() => { // same as running from event handler
      viewModel.lat = lat; // forceUpdate()
      viewModel.lon = long; // forceUpdate()
    }) // render()
    success = await store.update(viewModel)    
    ReactDOM.unstable_batchedUpdates(() => { // same as running from event handler
      if (success) {
        viewModel.status = "OK"; // forceUpdate()
        viewModel.isOpen = false; // forceUpdate()
      }      
    }) // render()   
}

[devuxer]

Thanks, @urugator

I didn't realize code in async functions that comes before the await did not get batched.

If I'm understanding you correctly, wouldn't this example from the docs cause unnecessary updates and renders? The first two lines of the async function are not wrapped with runInAction.

    @observable githubProjects = []
    @observable state = "pending" // "pending" / "done" / "error"

    @action
    async fetchProjects() {
        this.githubProjects = [] // not batched? forceUpdate()?
        this.state = "pending" // not batched? forceUpdate()?
        try {
            const projects = await fetchGithubProjectsSomehow()
            const filteredProjects = somePreprocessing(projects)
            // after await, modifying state again, needs an actions:
            runInAction(() => {
                this.state = "done"
                this.githubProjects = filteredProjects
            })
        } catch (error) {
            runInAction(() => {
                this.state = "error"
            })
        }
    }
}

// same as running from event handler

So, let's say I have this:

onChange={handleUpdateBeginTime} // react event thandler

...

function handleUpdateBeginTime(e, value) {
    viewModel.beginTime = value;
    viewModel.endTime = value + 60;   
}

Would this result in just one update and render (even though I have no action or ReactDOM.unstable_batchedUpdates)?

[urugator]

If I'm understanding you correctly, wouldn't this example from the docs cause unnecessary updates and renders?

To the best of knowledge, yes. The best way to be sure is to try out :)

Would this result in just one update and render

Yes, the code is wrapped in unstable_batchedUpdates automatically by react. See it here. The same is true for async function, but similary to mobx, react can't batch deferred updates. Be aware it's an implementation detail and can change in the future. Btw there is another problem with async event handlers. You can't access event object in them, because events are pooled. The react will throw if you attempt to do so.

[devuxer]

@urugator

Okay, that's great information, thanks again.

Here's my attempt at a takeaway from this thread:

It sounds like the main benefit of using action/runInAction is that it allows code that causes state changes to be explicitly marked and helps with debugging.

For simpler use cases where you don't have autoruns and reactions, and most of your state changes are simple and originate from react event handlers, using action/runInAction purely for performance reasons is probably "premature optimization". It still makes sense if you're getting value from the design or debugging benefits, but if you're not, it may not be worth the extra work and added code clutter.

The one case where I think I'd definitely want to wrap a function in an action is if I'm changing state within a loop or am setting a bunch of observables in a row. Here, I would expect a more significant performance benefit.

Would you agree with this, @urugator?

[urugator]

Most likely you won't be able to avoid the use of flow/action completely. In which case there is a choice: You can use it mindlessly everywhere and being sure that everything works optimally, at the cost of a little extra code. Which is very easy to explain to your colleagues and adopt as convention. Or you (and everyone else) have to keep thinking about whether it should be used or not, requiring deeper knowledge of react/mobx.

[devuxer]

@urugator

Fair point, but for some reason, I haven't found the use of action/runInAction to be mindless. The two things I find very awkward are (1) async code and (2) simple setters (like onChange={(e, value) => (viewModel.foo = value)}). Neither of these can be handled with just a simple decorator at the top of a function. In the case of (2), I could write a bunch of mindless functions like @action.bind function setFoo(value) { this.foo = value; }, but I'm struggling to see how this is better than just not using actions in this case.

[urugator]

async code

Replace async function() {} with flow(function *() {}) and await with yield, that's it. Even without mobx actions, you won't be able to use async function, for the reasons stated above (events/dom etc can't be safely accessed from deferred context and setState/forceUpdate can't be batched). The flow is the best you will get with async code.

simple setters

I suggest to wrap every state mutation into a method because of

• consistency
• extensibility (setter can be limiting)
• descriptiveness, clear of intend: Actions map to real use cases - these are expressed as verbs. Use names that describe the use case like onFooChanged(), setFoo is usually useless artificial abstraction. Design models around the UI requirements, the UI is the only reason for their existence.

If you insist on inlined form, you can wrap it in action as anywhere: onChange={action((e, value) => (viewModel.foo = value))} If you forget to do so, not a big deal (and Mobx can warn you with enforceActions option)

[devuxer]

@urugator,

Okay, I think I'm getting the picture now. Thanks for taking the time to answer all my questions. Flow definitely sounds like it will simplify things a lot for async store functions, so I guess that balances out some of the boilerplate needed to deal with property setters.

[devuxer]

Well, flow doesn't quite work as advertised in a TypeScript project.

Luckily, someone wrote a decorator

@urugator, Any idea why this is not built into mobx?

[urugator]

Babel has problems with decorator + generator. Check https://github.com/mobxjs/mobx-utils/issues/70

[PavanGangireddy]

If I'm understanding you correctly, wouldn't this example from the docs cause unnecessary updates and renders?

To the best of knowledge, yes. The best way to be sure is to try out :)

Would this result in just one update and render

Yes, the code is wrapped in unstable_batchedUpdates automatically by react. See it here. The same is true for async function, but similary to mobx, react can't batch deferred updates. Be aware it's an implementation detail and can change in the future. Btw there is another problem with async event handlers. You can't access event object in them, because events are pooled. The react will throw if you attempt to do so.

As per the above discussion, for the below snippet when the user presses "Get user details" button,

3 update requests - 3 renders should happen

instead of

3 update requests - 2 renders are reflected

import React, { Component } from "react";
import { render } from "react-dom";
import { observable } from "mobx";
import { observer } from "mobx-react";

@observer
class UserProfile extends Component {
  @observable userDetails = null;
  @observable count = 0;
  @observable secondCount = 0;

  // Mobx level batching - 3 update requests - 2 renders
  getUserDetails = async () => {
    this.count = this.count + 1; // forceUpdate() -> render()
    this.secondCount = this.secondCount + 1; // forceUpdate() -> render()
    let promise = new Promise((resolve, reject) => {
      resolve({
        name: "Tom Preston-Werner",
        company: "Facebook"
      });
    });
    let response = await promise;
    this.userDetails = response; // forceUpdate() -> render()
  };

  render() {
    console.log("render UserProfile");
    if (this.userDetails === null) {
      return <button onClick={this.getUserDetails}>Get user details</button>;
    }
    return (
      <div>
        {this.userDetails !== null && (
          <React.Fragment>
            <p>Name: {this.userDetails.name}</p>
            <p>Company: {this.userDetails.company}</p>
          </React.Fragment>
        )}
        <div>Count: {this.count}</div>
        <div>SecondCount: {this.secondCount}</div>
        <button onClick={this.getUserDetails}>Get user details</button>
      </div>
    );
  }
}

render(<UserProfile />, document.getElementById("root"));

Why? explanation needed @urugator

[urugator]

The async function runs "synchronously" until it hits the first await, so the first the 2 updates are probably batched by react (as they run from within an event handler). I was previously convinced that async function is immediately deferred (at it's first line) based on some experimenting I did in the past. Either async impl changed over time or I could just made some mistake. Btw react's concurrent mode should be able to batch "everywhere" https://reactjs.org/docs/concurrent-mode-adoption.html#feature-comparison

[PavanGangireddy]

Thanks, @urugator for the quick reply. I will close the new issue. Will look into the concurrent mode.