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

State update not happening when using after + wrapReduce #96

Closed JulianBissekkou closed 3 years ago

JulianBissekkou commented 3 years ago

After a network call, we dispatch another action that set the user login status in our appstate. This state update is never received by the UI.

This is the login action:

class LoginSubmitLoginAction extends BaseAction {
  @override
  void before() => dispatch(LoginSetLoadingAction(isLoading: true));

  @override
  void after() => dispatch(LoginSetLoadingAction(isLoading: false));

  @override
  Future<GlobalAppState> reduce() async {
    await Future<void>.delayed(const Duration(seconds: 3)); // service call
    dispatch(GlobalSetUserLoggedInAction(isUserLoggedIn: true)); // this action has no effect
    onSuccess();
    return state;
  }
}

As you can see, we dispatch GlobalSetUserLoggedInAction which has no effect on the ui. Edit: It works if this action does not extend BaseAction, but ReduxAction<GlobalAppState>! This is the action:

class GlobalSetUserLoggedInAction extends BaseAction {
  final bool isUserLoggedIn;

  GlobalSetUserLoggedInAction({@required this.isUserLoggedIn});

  @override
  GlobalAppState reduce() {
    return state.copyWith(isUserLoggedIn: isUserLoggedIn);
  }
}

We also have BaseAction which does error handling for us and it looks like this causes the issue. Here is the file:

abstract class BaseAction extends ReduxAction<GlobalAppState> {
  OnError onError;

  @override
  Reducer<GlobalAppState> wrapReduce(Reducer<GlobalAppState> reduce) {
    return () async {
      try {
        return await reduce();
      } catch (error, stackTrace) {
        onError?.call(error);
        errorWasThrown(error, stackTrace);
        return null;
      }
    };
  }

  /// This method can be overridden to get notified if an error was thrown.
  @protected
  void errorWasThrown(Object error, StackTrace stackTrace) {}
}

Not sure if you need this, but here is the StoreConnector that we use:

  @override
  Widget build(BuildContext context) {
    return StoreConnector<GlobalAppState, bool>(
      converter: (store) => store.state.isUserLoggedIn,
      builder: (context, isUserLoggedIn) {
        return Text("$isUserLoggedIn");
      },
    );
  }

It looks like the new state is discarded in _calculateModel as you see in this screenshot: Take a look at the two variables in the debugger. state is correct, but _forceLastValidStreamState ?? widget.store.state is the old state.

image

If you need any more details let me know! Great job with the package, really enjoying it so far 👍

marcglasberg commented 3 years ago

You really shouldn't be doing this. You are swallowing all errors thrown by all your reducers. You also seem to be turning all your reducers into async ones. I suggest you completely remove the wrapReduce and the BaseAction.

If you want to log errors, which it's what I think you want, please use the ErrorObserver:

var store = Store<AppState>(
  initialState: AppState.initialState(),
  errorObserver: MyErrorObserver<AppState>(),
);

class MyErrorObserver<St> implements ErrorObserver<St> {
  @override
  bool observe(Object error, StackTrace stackTrace, ReduxAction<St> action, Store store) {
    print("Error thrown during $action: $error");
    return true;
  }
}     
JulianBissekkou commented 3 years ago

Thanks for the fast response. We are not doing this for logging, we are doing this to reduce boilerplate code.

Previously, we had an onError callback in some of our actions so the view can handle exceptions and display the correct error message.

Here is the more boilerplate example:

class MyAction {
    final void Function(Object error) onError;
    MyAction(this.onError);

    Future<AppState> reduce() async {
      try {
        await someService.doStuff();
       } catch(e) {
         this.onError(e);
       }
      return null;
    }
}

This was one way of doing error handling in the previous "vanilla" redux implementation.

JulianBissekkou commented 3 years ago

Never mind, just saw that _store.dispatchFuture and _store.dispatch throws the error and there is no need for a callback in the action.