Returning a completed future from an async reducer

[foodchaining]

Hi, I see there is a “One important rule” requirement for an async reducer to return only uncompleted futures (which is related to ordering of state changes).

Instead of this requirement, wouldn't it be possible to always make the returned future uncompleted — at the library side, not in the application? May be this way, by changing _applyReducer (not tested):

FutureOr<void> _applyReducer(ReduxAction<St> action, {bool notify = true}) {
  _reduceCount++;

  var result = action.wrapReduce(action.reduce)();

  if (result is Future<St>) {
    Future<St> uncompleted = (() async => await result)();
    return uncompleted.then((state) => _registerState(
          state,
          action,
          notify: notify,
        ));
  } else if (result is St || result == null) {
    _registerState(result, action, notify: notify);
  } else
    throw AssertionError();
}

[foodchaining]

The above variant of _applyReducer will not help since the reducer was already invoked? Correct me if I am wrong: the primary peculiarity here is not in the future being uncompleted per se, but in the necessity of postponed invocation of the reducer, if it is asynchronous, so that the reducer and its returned future's then function would run in the same microtask.

[marcglasberg]

Reducers should generally create a copy of the state with some change in it. AsyncRedux has to apply this new state right away as soon as it gets it. If the Future is completed it will wait for the next microtask. But some other process may change the state before the state is applied, and then, when the state is indeed applied it may override the other processe's change.

I believe there should be a way for framework creators to know if a Future is completed or not, but Dart designers decided otherwise. They believe this information may be misused. I agree it can be misused, but then they could make it hard, not impossible to get this information. Anyway, there is not much I can do, since that's a language flaw (in my opinion).

This is the relevant test if you want to study the question:

https://github.com/marcglasberg/async_redux/blob/master/test/sync_async_test.dart

[foodchaining]

According to the documentation, synchronous reducers should be defined in an application using this signature: St reduce(). So async_redux can use this signature to detect if the reducer is synchronous or not and act differently. In the case of asynchronous reducer, it is possible, inside _applyReducer function, to call the reducer not directly but from a trivial async proxy function with await inside its body. Will this solve the problem, what do you think?

  FutureOr<void> _applyReducer(ReduxAction<St> action, {bool notify = true}) {
    _reduceCount++;

    var reducer = action.wrapReduce(action.reduce);

    if (reducer is St Function()) {
      St result = reducer();
      _registerState(result, action, notify: notify);
    } else {
      Future<St> result = (() async => await reducer())();
      return result.then((state) => _registerState(
            state,
            action,
            notify: notify,
          ));
    }
  }

[marcglasberg]

Yes, I've been checking the returned value type, but you have a point that maybe I could just check the function type itself. It's a good idea, I will give it a try.

If this works it will be a breaking change. At the moment, the beginning of the reducer (before the first await) runs synchronously. If your proposed change works, even that beginning would run asynchronously. However, I think it would be worth it.

[marcglasberg]

This doesn't work:

Future<St> result = (() async => await reducer())();

Because waiting for a completed future is also a completed Future.

But this works:

Future<St> result = (() async {
     await Future.microtask(() {});
     return reducer();
})();

[marcglasberg]

@foodchaining Please see version 4.0 (in GitHub. Not published to pub.dev) and tell me what you think.

[foodchaining]

So a mere await was not enough and you added an explicit microtask, very well! The only minor thing that I think could be improved here is readability: the third condition check appears to be redundant and gives an impression that there might be a fourth variant of a reducer.

[saibotma]

Reducers should generally create a copy of the state with some change in it. AsyncRedux has to apply this new state right away as soon as it gets it. If the Future is completed it will wait for the next microtask. But some other process may change the state before the state is applied, and then, when the state is indeed applied it may override the other processe's change.

I believe there should be a way for framework creators to know if a Future is completed or not, but Dart designers decided otherwise. They believe this information may be misused. I agree it can be misused, but then they could make it hard, not impossible to get this information. Anyway, there is not much I can do, since that's a language flaw (in my opinion).

This is the relevant test if you want to study the question:

https://github.com/marcglasberg/async_redux/blob/master/test/sync_async_test.dart

Is there an issue in the dart repository regarding this? Would love to give it thumbs up.

[saibotma]

Is the One important rule actually still required? I could not find it in the current documentation on master branch. I had an issue where two actions of mine executed in the same Future.wait did override each others returned state when I did not follow the rule.

This is how the actions looked like without following the rule:

class MyAction {
  @override
  Future<RootState?> reduce() async {
    final value = condition
        ? await calculateFuture()
        : null;

    return state.copyWith(value: value)
  }
}

This is how they now look like with following the rule:

class MyAction {
  @override
  Future<RootState?> reduce() async {
    final value = await (condition
        ? calculateFuture()
        : null);

    return state.copyWith(value: value)
  }
}

I dispatched those actions like the following, and one of the two actions always swallowed the state change of the other: await Future.wait([dispatchAsync(MyAction1()), dispatchAsync(MyAction2())])

[marcglasberg]

@saibotma Just make sure to return AppState? if you want the reducer to be sync, and return Future<AppState?> if you want it to be async.

I don't understand your question about overriding state:

• If you dispatch 2 async actions that change the SAME value, both will execute, but the one that executes later will "win" in the end.
• If you dispatch 2 async actions that change DIFFERENT values, then both will execute, and the end state will have both modifications, after both run.

BTW, another way to write your action above is:

class MyAction {
  @override
  Future<RootState?> reduce() async => 
        condition 
              ? state.copyWith(value: await calculateFuture());
              : null;
  }

[saibotma]

I've created a follow-up issue for this: #131