marcglasberg / async_redux

Flutter Package: A Redux version tailored for Flutter, which is easy to learn, to use, to test, and has no boilerplate. Allows for both sync and async reducers.
Other
230 stars 41 forks source link

Code calls return after handling first item of iterable passed to isWaiting #163

Closed swrenn closed 3 weeks ago

swrenn commented 3 weeks ago

Source

    else if (actionOrActionTypeOrList is Iterable) {
      for (var actionOrType in actionOrActionTypeOrList) {
        if (actionOrType is Type) {
          _awaitableActions.add(actionOrType);
          return _actionsInProgress.any((action) => action.runtimeType == actionOrType);
        } else if (actionOrType is ReduxAction) {
          _awaitableActions.add(actionOrType.runtimeType);
          return _actionsInProgress.contains(actionOrType);
        } else {
          Future.microtask(() {
            throw StoreException(
                "You can't do isWaiting([${actionOrActionTypeOrList.runtimeType}]), "
                "but only an action Type, a ReduxAction, or a List of them.");
          });
        }
      }
      return false;
    }

Also, thanks again for all the work on this library.

swrenn commented 3 weeks ago

Also, remove is never called on _awaitableActions. Looks like it might be complicated to do that, though.

marcglasberg commented 3 weeks ago

Hi @swrenn it seems to me the code is correct. Could you please explain a little bit more what the problem is?

The isWaiting should return true if any of the actions is found to match. Here is a commented code:

else if (actionOrActionTypeOrList is Iterable) {

      // 1. For each action or action type in the iterable...
      for (var actionOrType in actionOrActionTypeOrList) {

        // 2. If it's a type.
        if (actionOrType is Type) {
          _awaitableActions.add(actionOrType);

          // 2.1. Return true if any of the actions in progress has that exact type.
          return _actionsInProgress.any((action) => action.runtimeType == actionOrType);
        } 
        // 
        // 3. If it's an action.
        else if (actionOrType is ReduxAction) {
          _awaitableActions.add(actionOrType.runtimeType);

          // 3.1. Return true if any of the actions in progress is the exact action.
          return _actionsInProgress.contains(actionOrType);
        } 
        // 
        // 4. If it's not an action and not an action type, throw an exception.
        // The exception is thrown after the async gap, so that it doesn't interrupt the processes.
        else {
          Future.microtask(() {
            throw StoreException(
                "You can't do isWaiting([${actionOrActionTypeOrList.runtimeType}]), "
                "but only an action Type, a ReduxAction, or a List of them.");
          });
        }
      }

      // 5. If the for finished without matching any items, return false (it's NOT waiting).
      return false;
    }
marcglasberg commented 3 weeks ago

Regarding _awaitableActions, is just an optimization. It's supposed to list all actions types that could at some point be tested by isWaiting, so that we optimize by NOT testing actions that could not, in principle, result in isWaiting true. Note it only contains types, not actions (it could have been calling _awaitableActionTypes.

swrenn commented 3 weeks ago

With the early return, not all types are added to _awaitableActions. Downstream, _changeController is not notified for the types that are not added, meaning any VmFactory that calls isWaiting([Action1, Action2]) will not rebuild its Vm with a new value for isWaiting when Action2 is dispatched.

Downstream uses of _awaitableActions:

_calculateIsWaitingIsFailed

    if (!theUIHasAlreadyUpdated && _awaitableActions.contains(action.runtimeType)) {
      _changeController.add(state);
    }

_finalize

    if (_awaitableActions.contains(action.runtimeType) && ((error != null) || !notify)) {
      _changeController.add(state);
    }
marcglasberg commented 3 weeks ago

You are right. Thanks, I will fix this.

marcglasberg commented 3 weeks ago

Fixed for version [23.2.0]. Thanks again!