Returned state gets overridden when returning an uncompleted Future

[saibotma]

This is a follow-up issue to #86.

Steps to reproduce:

• Run the example below.
• Press the "Increment nums" button
• Notice that only Num1 is incremented.
• Change order of dispatched actions in ParentAction according to the comments.
• Notice that the increment behaviour changes.
• Set condition to true in OtherAction.
• Notice that all increments issues disappear.

Actually I hope that I am just using async_redux or futures fundamentally wrong, however in case I am doing this right, then this might be a serious issue.

async_redux: 14.1.5 flutter: 3.0.0

Code

```dart import 'dart:async'; import 'package:async_redux/async_redux.dart'; import 'package:flutter/material.dart'; class AppState { final int num1; final int num2; AppState({required this.num1, required this.num2}); AppState copyWith({int? num1, int? num2}) { return AppState( num1: num1 ?? this.num1, num2: num2 ?? this.num2, ); } } class ViewModel extends Vm { final int num1; final int num2; ViewModel({ required this.num1, required this.num2, }) : super(equals: [num1, num2]); } class IncrementNum1Action extends ReduxAction { @override Future reduce() async { // Imagine that the increment is an async call. final newNum = await Future.value(state.num1 + 1); return state.copyWith(num1: newNum); } } class IncrementNum2Action extends ReduxAction { @override Future reduce() async { // Imagine that the increment is an async call. final newNum = await Future.value(state.num2 + 1); return state.copyWith(num2: newNum); } } class OtherAction extends ReduxAction { @override Future reduce() async { // Imagine this to be a conditional that can be false sometimes, based // on state. If you set this to true, then the increments work without // problems. const condition = false; if (condition) { await Future.delayed(const Duration(milliseconds: 100)); } return state.copyWith(); } } class ParentAction extends ReduxAction { @override Future reduce() async { await Future.wait([ // Num1 is incremented. dispatchAsync(IncrementNum1Action()), dispatchAsync(OtherAction()), // Num2 is not incremented, because the action is after OtherAction // in the array. If you put OtherAction last, then none of the both // nums are incremented. dispatchAsync(IncrementNum2Action()), ]); return null; } } late Store store; void main() { store = Store(initialState: AppState(num1: 0, num2: 0)); runApp(const MyApp()); } class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return StoreProvider( store: store, child: MaterialApp( home: StoreConnector( converter: (store) { return ViewModel( num1: store.state.num1, num2: store.state.num2, ); }, builder: (context, vm) { return Column( mainAxisAlignment: MainAxisAlignment.center, children: [ ElevatedButton( onPressed: () { StoreProvider.dispatch(context, ParentAction()); }, child: const Text("Increment nums"), ), Text("Num1: ${vm.num1}"), Text("Num2: ${vm.num2}"), ], ); }, ), ), ); } } ```

[marcglasberg]

The "One important rule" was relaxed because a contributor found a hack that depended on implementation details of Futures, which were not part of the API, but are not valid for the current Future implementation. I apologize that I didn't notice this. You are right, that rule must be applied. I will publish a new version and put the "One important rule" back into the documentation. This means if your action returns a Future it must have an await:

class OtherAction extends ReduxAction<AppState> {
   Future<AppState?> reduce() async {
      await microtask;
      return state.copyWith();
}}

Or else, it must not return a Future:

class OtherAction extends ReduxAction<AppState> {
   AppState? reduce() => state.copyWith();
}}

Some more detail, in case you are interested:

When a reducer returns a state, we need to apply that state immediately in the store. If we wait even a single microtask, another reducer may change the store-state before we have the chance to apply the state. This would result in the later reducer overriding the value of the other reducer, and state changes will be lost. To fix this we'll depend on the behavior described below, which was confirmed by the Dart team:

1) When a future returned by an async function completes, it will call the then method synchronously (in the same microtask), as long as the function returns a value (not a Future) AND this happens AFTER at least one await. This means we then have the chance to apply the returned state to the store right away.

2) When a future returned by an async function completes, it will call the then method asynchronously (delayed to a later microtask) if there was no await in the async function. When that happens, the future is created "completed", and Dart will wait for the next microtask before calling the then method (they do this because they want to enforce that a listener on a future is always notified in a later microtask than the one where it was registered). This means we will only be able to apply the returned state to the store during the next microtask. There is now a chance state will be lost. This situation must be avoided at all cost, and it's actually simple to solve it: An async reducer must never complete without at least one await. Unfortunately, if the developer forgets to add the await, there is no way for AsyncRedux to let them know about it, because there is no way for us to know if a Future is completed. The completion information exists in the FutureImpl class but it's not exposed. I have asked the Dart team to expose this information, but they refused. The only solution is to document this and trust the developer. (Or I will try to create a linter rule to detect this and show a warning warn).

Important: The behavior (1) described above was confirmed by the Dart team, but it's NOT documented. In other words, they make no promise that it will be kept in the future. If that ever changes, AsyncRedux will need to change too, to return St? Function(state) instead of returning state. For example, instead of a reducer ending with return state.copy(123) it will have to return (state) => state.copy(123).

[saibotma]

Thanks for the detailed answer. This was very interesting to read.

[marcglasberg]

Just published version 16.0.0 to GitHub. It returns the rule to the docs: https://github.com/marcglasberg/async_redux/#one-important-rule. I should publish it to pub.dev during next week.

Also, please see the changelog: https://github.com/marcglasberg/async_redux/blob/master/CHANGELOG.md