mobx.dart
mobx.dart copied to clipboard
Fix: AsyncAction behavior
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.
- New behavior:
/// The same behavior with mobx >= 2.0.7+3
abstract class _YourStore with Store {
@override
ReactiveContext get context {
return mainContext
..config.clone(asyncActionBehavior: AsyncActionBehavior.notifyOnlyLast);
}
}
- Old behavior (currently users don't need to add this code):
/// The same behavior with mobx <= 2.0.7+2
abstract class _YourStore with Store {
@override
ReactiveContext get context {
return mainContext
..config.clone(asyncActionBehavior: AsyncActionBehavior.notifyEachNested);
}
}
Fixes: #834 Fixes: #836 Fixes: #840
Pull Request Checklist
- [x] If the changes are being made to code, ensure the version in
pubspec.yamlis updated. - [x] Increment the
major/minor/patch/patch-count, depending on the complexity of change - [x] Add the necessary unit tests to ensure the coverage does not drop
- [x] Update the Changelog to include all changes made in this PR
- [x] Run the
set:versionscommand usingnpmoryarn. You can find this command in thepackage.jsonfile in the root directory - [ ] Include the necessary reviewers for the PR
- [ ] Update the docs if there are any API changes or additions to functionality
Deploy Preview for mobx canceled.
| Name | Link |
|---|---|
| Latest commit | b8573c4da7d6d0bfe81e07a3635194a38621c159 |
| Latest deploy log | https://app.netlify.com/sites/mobx/deploys/62c46a995e09100008915a7d |
@vursin Thank you for your contribution. I think #784 should be reverted(#842). After that, we need to discuss the async action.
@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.
Hi all,
Does this just need a final review before we can merge and close it?
Thanks, Gavin.
@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() {
}
@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() { }