mobxjs / mobx.dart

MobX for the Dart language. Hassle-free, reactive state-management for your Dart and Flutter apps.
https://mobx.netlify.app
MIT License
2.4k stars 310 forks source link

Fix: AsyncAction behavior #841

Closed lamnhan066 closed 1 year ago

lamnhan066 commented 2 years ago

Hello,

I found this issue when I test my last PR. After doing some searching, I found that the PR #784 shouldn't be introduced without a breaking change version.

So I decided to fix this issue without breaking the code of the old users (who may be used this package since 2019 with PR #89) and allows anyone who wants to use new behavior (which is the official behavior of Mobx in the document).

I add an asyncActionBehavior flag to ReactiveConfig to control the behavior of @action on Future methods. This config is just needed for those who want to use new behavior to avoid breaking old users' code.

Fixes: #834 Fixes: #836 Fixes: #840


Pull Request Checklist

netlify[bot] commented 2 years ago

Deploy Preview for mobx canceled.

Name Link
Latest commit b8573c4da7d6d0bfe81e07a3635194a38621c159
Latest deploy log https://app.netlify.com/sites/mobx/deploys/62c46a995e09100008915a7d
amondnet commented 2 years ago

@vursin Thank you for your contribution. I think #784 should be reverted(#842). After that, we need to discuss the async action.

lamnhan066 commented 2 years ago

@amondnet Additional information: This PR has the same behavior as reverted #784, just add an additional flag for those who want to continue using the new behavior.

ghenry commented 1 year ago

Hi all,

Does this just need a final review before we can merge and close it?

Thanks, Gavin.

amondnet commented 1 year ago

@vursin Hello and thanks for the PR!

However, I'm not sure if we need this feature. Are there cases where this feature is needed? How often is that case? whole app, some store or some method? Does this pr solve that case?

How about something like below?

@action(batch=true) //
Future<void> runInBatch() {
}
lamnhan066 commented 1 year ago

@amondnet I think this feature is also not really needed myself but it will be better if there is a way to just notify when all the actions inside are finished.

I also think this is a better way to do that so feel free to close this PR.

How about something like below?

@action(batch=true) //
Future<void> runInBatch() {
}