using ts disposable

[vovsemenv]

implements #3840

Code change checklist

• Added/updated unit tests
• Updated /docs. For new functionality, at least API.md should be updated
• Verified that there is no significant performance drop (yarn mobx test:performance)

[changeset-bot]

🦋 Changeset detected

Latest commit: 3d1b0e5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mobx	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[kubk]

@vovsemenv Thank you, could you add some unit tests?

[kubk]

@mweststrate @urugator I think having a utility like this is a good option, as the other ways to modify code asynchronously are far from ideal:

• runInAction with async/await increases code nesting.
• Mobx flow requires a hack to have proper TypeScript support. Fully typed flow idea #3195

[vovsemenv]

@kubk added test, please approve ci run

[urugator]

I am not strictly opposed, but I think it has some negatives, while I am not sure about benefits.

1. Second way to do the same thing.
2. How will it play with eslint? I would suspect using becomes a subject of no-unused-vars or no-unused-resource will be introduced. This would require an extra convention and configuration.
3. It feels like bending something that was designed for something else. The code is literally saying "use this resource" and then you're not using it. Could be wrong about this, I could imagine resource that only has a side effect like spinning up a remote server. The point is, it's designed for resource disposal, not for intercepting.
4. It can't intercept error or return value. We don't need that now, but something to keep in mind.

runInAction with async/await increases code nesting.

Isn't it almost the same?

async action() {
  using _ = _action();  
  this.x = 1
  await fetch()
  {
    using _ = _action();
    this.x = 1
  } 
  await fetch()
  {
    using _ = _action();
    this.x = 1;
  }
}

Am I missing something?

[kubk]

@urugator Valid points have been brought up 👍 Yes, the requirement of having a variable leads to issues with ESLint.

@vovsemenv are you interested in moving it to userland as a separate NPM package? It seems like the solution has its own drawbacks and can't be added to the core.

[mweststrate]

It is kinda cool! But a crucial design question is still open I think; transactions in MobX are always concluded synchronously (otherwise it might interfere with concurrent transactions or 'block' updating entirely). So given something like:

const counter = observable({count : 1, inc() { this.count++; } })

autorun(() => console.log(counter.count))

things();

async function things() {
  using _ = action();
  counter.inc()
  counter.inc()

  await sleep(100)
  console.log("slept")

  counter.inc()
  counter.inc()
}

I'd expect the output to be:

2
slept
4

But I think that will be the case with the current implementation, as the updates will only appear entirely at the end, and the output is just slept and 4?

The reason why that is important, is that a) transactions are global in MobX and b) we don't want to block things like react components updating in an async way, as in that case the app might freeze if you are doing a async transaction that contains a network request or so.

[urugator]

as the updates will only appear entirely at the end

You're correct. I thought there is a difference between using and await using, but both are disposed at the end of the block. So the example I posted above would have to look like this:

async action() {
  {
    using _ = _action();
    this.x = 1
  } 
  await fetch()
  {
    using _ = _action();
    this.x = 1
  } 
  await fetch()
  {
    using _ = _action();
    this.x = 1;
  }
}

In principle you can combine it with annotation/decorator:

@action
async action() {
  this.x = 1;  
  await fetch()
  {
    using _ = _action();
    this.x = 1
  } 
  await fetch()
  {
    using _ = _action();
    this.x = 1;
  }
}

[mweststrate]

Closing this PR as there seems sadly no way to make this work for async boundaries, and without it will just be another way (not particularly easier or shorter) of doing runInAction. Thanks for trying this cool idea though!