fluttercommunity / redux.dart

Redux for Dart
https://pub.dev/packages/redux
MIT License
515 stars 61 forks source link

TypedMiddleware not accept specified middleware type. #35

Closed jaggerwang closed 6 years ago

jaggerwang commented 6 years ago

Problem

When specify the exact action type, TypedMiddleware complains [dart] The return type '(Store<AppState>, LoginAction, (dynamic) → void) → Null' isn't a '(Store<AppState>, dynamic, (dynamic) → void) → void', as defined by the method 'loginMiddleware'..

Code

List<Middleware<AppState>> createMiddlewares({
  AccountService accountService,
}) {
  accountService = accountService ?? AccountService();

  return [
    TypedMiddleware<AppState, LoginAction>(loginMiddleware(accountService)),
  ];
}

Middleware<AppState> loginMiddleware(AccountService accountService) =>
    // Specify the exact type of action, not dynamic
    (Store<AppState> store, LoginAction action, NextDispatcher next) {
      next(action);
    };

Env

name: tangbole
description: A tumblr client app.

dependencies:
  cupertino_icons: ^0.1.2
  english_words: ^3.1.0
  flutter:
    sdk: flutter
  flutter_redux: ^0.5.1
  http: ^0.11.3+16
  meta: ^1.1.5
  uri: ^0.11.2
  redux: ^3.0.0
  redux_logging: ^0.3.0
  redux_persist: ^0.7.0-rc.2
  redux_persist_flutter: ^0.6.0-rc.1

dev_dependencies:
  flutter_driver:
    sdk: flutter
  flutter_test:
    sdk: flutter
  mockito: ^2.2.3

flutter:
  uses-material-design: true
  assets:
    - images/lake.jpg
brianegan commented 6 years ago

What if you try this? This is similar to your other problem: Dart is inferring the more specific type information that TypedMiddleware provides rather than the more abstract Middleware<AppState>.

List<Middleware<AppState>> createMiddlewares({
  AccountService accountService,
}) {
  accountService = accountService ?? AccountService();

  return <Middleware<AppState>>[
    TypedMiddleware<AppState, LoginAction>(loginMiddleware(accountService)),
  ];
}
jaggerwang commented 6 years ago

Because I want use code assistant for action! Using type dynamic lost the exact action type information, and there is no code assistant.

The action type param is already given to TypedMiddleware, so why not using the exact middleware type?

brianegan commented 6 years ago

I'd ask: Please let us be kind and patient with one another. I'm doing my best to help you with the very limited amount of free time I have. In this case, your app has a small type error. Could you please try this and see if it works?

I realized the mistake is in a slightly different part of your code. Please change this:

Middleware<AppState> loginMiddleware(AccountService accountService) =>
    // Specify the exact type of action, not dynamic
    (Store<AppState> store, LoginAction action, NextDispatcher next) {
      next(action);
    };

to this:


void Function(
    Store<State> store,
    LoginAction action,
    NextDispatcher next,
  ) loginMiddleware(AccountService accountService) =>
    (Store<AppState> store, LoginAction action, NextDispatcher next) {
      next(action);
    };

In this case, you want to return a Function, but you don't want to return a Middleware<AppState> function because it does not match what TypedMiddleware is expecting (TypedMiddleware does not accept a Middleware<AppState> function).

In this case, you need to conform to what the TypedMiddleware constructor expects: A function with more specific type info.

jaggerwang commented 6 years ago

Sorry, I read this "What if you try this?" to "Why if you try this?":)

Anyway, using the Function way works, but it's not a elegant way. Right now I'm using the following way.

Middleware<AppState> loginMiddleware(AccountService accountService) =>
    (Store<AppState> store, dynamic action, NextDispatcher next) {
      final a = action as LoginAction;
      accountService.login(a.username, a.password).then((response) {
        if (response.code == TangboleApiResponseCode.ok) {
          store.dispatch(
              LoginSucceededAction(UserModel.fromJson(response.data['user'])));
        } else {
          store.dispatch(NotifyAction(NoticeModel(message: response.message)));
        }
      });
    };
brianegan commented 6 years ago

No problem at all.

As a heads up: your current solution might lead to runtime errors. If the action is not a LoginAction, but a LogoutAction, you'll get a CastError when the Middleware attempts to convert a LogoutAction to a LoginAction on this line: final a = action as LoginAction;. Therefore, I'd recommend using an if statement: if (action is LoginAction) { /* rest of code here */ } or another option: Extend TypedMiddleware instead. It would look something like this (might need to be adjust slightly as I don't have your full class tree):

class MyMiddleware extends TypedMiddleware<AppState, LoginAction> {
  MyMiddleware(AccountService service)
      : super(
          (
            Store<AppState> store,
            LoginAction action,
            NextDispatcher next,
          ) {
            service.login(a.username, a.password).then((response) {
              if (response.code == 200) {
                store.dispatch(LoginSucceededAction(
                    UserModel.fromJson(response.data['user'])));
              } else {
                store.dispatch(
                    NotifyAction(NoticeModel(message: response.message)));
              }
            });
          },
        );
}
brianegan commented 6 years ago

Since we solved the original problem, I'll go ahead and close this issue. Please let me know if ya need more help :)

jaggerwang commented 6 years ago

There is another way.

List<Middleware<AppState>> createMiddlewares({
  AccountService accountService,
}) {
  accountService = accountService ?? AccountService();

  return [
    TypedMiddleware<AppState, LoginAction>(loginMiddleware(accountService)),
  ];
}

dynamic loginMiddleware(AccountService accountService) =>
    (Store<AppState> store, LoginAction action, NextDispatcher next) {
      accountService.login(action.username, action.password).then((response) {
        if (response.code == TangboleApiResponseCode.ok) {
          store.dispatch(
              LoginSucceededAction(UserModel.fromJson(response.data['user'])));
        } else {
          store.dispatch(NotifyAction(NoticeModel(message: response.message)));
        }
      });
    };

As TypedMiddleware make sure to pass the right type of action to loginMiddleware(), so we can simply using return type dynamic to allow specify the exact action type.

Dart not support generic param extending? As type (Store<AppState> store, LoginAction action, NextDispatcher next) is not a subtype of (Store<AppState> store, dynamic action, NextDispatcher next).

brianegan commented 6 years ago

"As TypedMiddleware make sure to pass the right type of action to loginMiddleware(), so we can simply using return type dynamic to allow specify the exact action type."

Sure, that works. I'd personally use the full type info rather than dynamic, but if ya like that better then go for it.

"Dart not support generic param extending?"

We might need to look at changing this -- In Dart 1, dynamic was a "base type" for all objects. In Dart 2, it isn't. Therefore, it might make a bit more sense to use Object instead. However, I'd have to test if that would resolve the type issue or if Dart would still complain.