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

View model does not rebuild immediately after action ends #68

Closed andreitere closed 4 years ago

andreitere commented 4 years ago

Hi there,

I have a login flow where I'm dispatching an action (Future) and then immediately trying to access the state (via model) after the action finishes. The strangest thing here is that this happens randomly. Sometimes the value is immediately available

Doing this in the widget:

await widget.vm.login("facebook", result.accessToken.token, "");

if (widget.vm.isLoggedIn == true) {
      _showErrorDialog("Login success",
          "Hello there. We will redirect you to the home page soon");
      Timer(Duration(seconds: 4), () {
        mainNavigator.currentState.pushNamed("/");
      });
    }
    if(widget.vm.isLoggedIn == false) {
      _showErrorDialog("Login fail",
          "Backend once again down 🤦");
    }

Seems that the model rebuilds "too late". I'm making sure that the state is mutated.

Should this scenario work fine?

Abstracted a bit my store setup is:

class AppState {
  final UserState userState;
  final AppEventsState appEventsState;
  final I18nState i18nState;

  AppState({this.userState, this.i18nState, this.appEventsState});

  AppState copy(
      {UserState userState,
      I18nState i18nState,
      AppEventsState appEventsState}) {
    return AppState(
      userState: userState ?? this.userState,
      i18nState: i18nState ?? this.i18nState,
      appEventsState: appEventsState ?? this.appEventsState,
    );
  }

  static AppState initialState() => AppState(
        userState: UserState.initialState(),
        i18nState: I18nState.initialState(),
        appEventsState: AppEventsState.initialState(),
      );

  @override
  bool operator ==(Object other) =>
      identical(this, other) ||
          other is AppState &&
              runtimeType == other.runtimeType &&
              userState == other.userState;

  @override
  int get hashCode => userState.hashCode;
}
class UserState {
  final bool loggedIn;
  final String userToken;
  final User user;
  final bool loginFailed;

  UserState({this.loggedIn, this.userToken, this.user, this.loginFailed});

  static UserState initialState() => UserState(
        loggedIn: false,
        userToken: "",
        user: null,
        loginFailed: false
      );

  UserState copy({loggedIn, userToken, user, loginFailed}) => UserState(
        loggedIn: loggedIn ?? this.loggedIn,
        userToken: userToken ?? this.userToken,
        user: user ?? this.user,
        loginFailed: loginFailed ?? this.loginFailed

      );

  @override
  bool operator ==(Object other) =>
      identical(this, other) ||
          other is UserState &&
              runtimeType == other.runtimeType &&
              loggedIn == other.loggedIn;

  @override
  int get hashCode => loggedIn.hashCode;
}

Model:

class LoginPageViewModel extends BaseModel<AppState> {
  LoginPageViewModel();

  bool isLoggedIn;
  Function login;

  LoginPageViewModel.build({
    @required this.isLoggedIn,
    @required this.login
  }) : super(equals: [isLoggedIn]);

  @override
  LoginPageViewModel fromStore() {
    return LoginPageViewModel.build(
      isLoggedIn: state.userState.loggedIn,
      login: (authGateway, userToken, uid) {
        return dispatchFuture(UserLoginAction(
          authGateway: authGateway,
          userToken: userToken,
          uid: uid,
        ));
      },
    );
  }
}

also the action


class UserLoginAction extends BaseAction {
  final String authGateway;
  final String userToken;
  final String uid;

  UserLoginAction({this.authGateway, this.userToken, this.uid});

  Preferences _prefs = Preferences.singleton();

  @override
  Future<AppState> reduce() async {
    APIResponse login = await BackboneService()
        .loginUser({"authGateway": authGateway, "userToken": userToken});
    if (login.errorCode != null) {
      return state.copy(userState: userState.copy(loginFailed: true));
    }
    await this._prefs.setLoginToken(login.result["authToken"]);
    return state.copy(
        userState: userState.copy(userToken: login.result["authToken"], loggedIn: true));
  }
}
import 'package:async_redux/async_redux.dart';
import 'package:flutter/material.dart';
import 'package:voyah/store/AppState.dart';

import 'LoginPageView.dart';
import 'LoginPageViewModel.dart';

class LoginPage extends StatelessWidget {
  LoginPage({Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return StoreConnector<AppState, LoginPageViewModel>(
      model: LoginPageViewModel(),
      builder: (BuildContext context, LoginPageViewModel vm) => LoginPageView(
        isLoggedIn: vm.isLoggedIn,
        vm: vm
      ),
    );
  }
}
marcglasberg commented 4 years ago

Are you accessing the viewmodel in the dumb-widget? What do you mean by "widget.vm"? This seems weird to me.

The view-model is private to the StoreConnector. Are you using a StoreConnector at all?

andreitere commented 4 years ago

Aplogies. Forgot to attach it.

import 'package:async_redux/async_redux.dart';
import 'package:flutter/material.dart';
import 'package:voyah/store/AppState.dart';

import 'LoginPageView.dart';
import 'LoginPageViewModel.dart';

class LoginPage extends StatelessWidget {
  LoginPage({Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return StoreConnector<AppState, LoginPageViewModel>(
      model: LoginPageViewModel(),
      builder: (BuildContext context, LoginPageViewModel vm) => LoginPageView(
        isLoggedIn: vm.isLoggedIn,
        vm: vm
      ),
    );
  }
}

Updated also the initial post Tried both ways passing down only the function. Here i passed the full vm for brevity

marcglasberg commented 4 years ago

Ok:

1) You should never pass the vm to the widget. Pass only the information you want. In this case, pass only isLoggedIn:

builder: (BuildContext context, LoginPageViewModel vm) => LoginPageView(
        isLoggedIn: vm.isLoggedIn,
      ),

2) The widget is surely rebulding. I think you can see that if you add a print to your StoreConnector's build method.

3) What is this suppose to be doing?

await widget.vm.login("facebook", result.accessToken.token, "");

You can't have awaits inside of your widgets. Is this code inside of the widget? How? Where?

4) I'd say the problem is not within the Redux part. It seems to me you don't have the right mental model of how widget rebuild works in general. Is your LoginPageView class a Stateless or a Statefull widget?

andreitere commented 4 years ago

1) Ok. Got it. As sad i tried also with passing only the information i want 2) I didn't say that it does not rebuild. Problem is that immediately after action ends i cannot use the property from the store 3) widget.vm.login is dispatching a future action (dispatchFuture) 4) I might have the wrong mindset here but wan't to check if that's really the case and the redux part si working correct. I also tested different approaches and want to confirm this also:).

I built a smaller example (full code here).

Bottom line is that after await widget.login() i want to access a boolean in the store to see the result of that login action.

class _MainPageViewState extends State<MainPageView> {
  _doSomething() async {
    print("calling action");
    await widget.login();
    print("Mutated prop is" + widget.isLoggedIn.toString());
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Container(
          child: Center(
        child: InkWell(
          onTap: () => _doSomething(),
          child: SizedBox(
            child: Container(
              color: Colors.blue,
              child: Text(
                "TapMe",
                style: TextStyle(color: Colors.white),
              ),
            ),
          ),
        ),
      )),
    );
  }
}

Some logs image

Thank you for your time :)

marcglasberg commented 4 years ago

1) You can put the viewmodel in the same file as the StoreConnector, and then make it private: _ViewModel instead of MainPageViewModel.

2) Your login should not return anything and it should use dispatch, not dispatchFuture:

`login: () { dispatch(UserLoginAction(authGateway: "test", userToken: "test", uid: "test")); },`

3) Your onTap should call widget.login:

`child: InkWell( onTap: widget.login,`

4) Your example is not good, because it's not actually using the changed state. Of course, the state will be the same after the wait in that _doSomething method. Remember the state is immutable, so it cannot change! You should use the changed state in the build method, instead.

In other words, Flutter will call the build method again with the new state, after the widget.login method is called.

Try it like this:

child: Text(
   "TapMe: ${widget.isLoggedIn}",

As I said, you have the wrong mental model. I wil repeat: You should not expect widget.isLogged to change after the await widget.login(); because this information is immutable. You yourself marked it final:

 final bool isLoggedIn;

What will happen is that the widget will be rebuild with a NEW, also immutable state, that contains the changed information.

Got it?

baoxiehao commented 4 years ago

Ok:

  1. You should never pass the vm to the widget. Pass only the information you want. In this case, pass only isLoggedIn:
builder: (BuildContext context, LoginPageViewModel vm) => LoginPageView(
        isLoggedIn: vm.isLoggedIn,
      ),
  1. The widget is surely rebulding. I think you can see that if you add a print to your StoreConnector's build method.
  2. What is this suppose to be doing?
await widget.vm.login("facebook", result.accessToken.token, "");

You can't have awaits inside of your widgets. Is this code inside of the widget? How? Where?

  1. I'd say the problem is not within the Redux part. It seems to me you don't have the right mental model of how widget rebuild works in general. Is your LoginPageView class a Stateless or a Statefull widget?

You should never pass the vm to the widget. Why? I think it will be quite inconvenient.

marcglasberg commented 4 years ago

@baoxiehao Maybe I should not have said never. You can. But then your widget depends on the ViewModel class. It's an unnecessary dependency. You have a widget that gets some information in its constructor and then you are putting this information into a class and passing the class instead.

Also, sometimes your widget gets only part of its information from the vm, and the rest is information that the connector got from its own constructor and is directly passing down to the dumb-widget.

Also, if you want to add fields to the dumb-widget you must not add it in its constructor, but instead add it to the vm class. So the construction information is not clear in the constructor, but instead it's hidden in the vm class, which is usually defined in another file. This is not clean code at all.

Also, a dumb widget may sometimes be used with more than one StoreConnector, which means more than one ViewModel class.