alibaba / fish-redux

An assembled flutter application framework.
https://github.com/alibaba/fish-redux
Apache License 2.0
7.33k stars 843 forks source link

Reducer is not called after subEffect #613

Open achernoprudov opened 4 years ago

achernoprudov commented 4 years ago

Describe the bug Lets assume that we have three different actions: begin, middle, end.

begin action is handled only by effect E1 that dispatches the middle action. middle action has effect E2 and reducer R2. E2 dispatches the end action. end action has only reducer R3.

So the problem is that if we dispatch begin action from view - R2 wont be called.

I made a project to demonstrate behaviour.

To Reproduce Write code below:

/// actions
enum MainAction { startAction, middleAction, endAction }
class MainActionCreator {
  static Action onStartAction() => Action(MainAction.startAction);
  static Action onMiddleAction() => Action(MainAction.middleAction);
  static Action onEndAction() => Action(MainAction.endAction);
}

/// effects
Effect<MainState> buildEffect() {
  return combineEffects(<Object, Effect<MainState>>{
    MainAction.startAction: _onStartAction,
    MainAction.middleAction: _onMiddleAction,
  });
}

void _onStartAction(Action action, Context<MainState> ctx) => ctx.dispatch(MainActionCreator.onMiddleAction());
void _onMiddleAction(Action action, Context<MainState> ctx) => ctx.dispatch(MainActionCreator.onEndAction());

/// reducers
Reducer<MainState> buildReducer() {
  return asReducer(
    <Object, Reducer<MainState>>{
      MainAction.middleAction: _onMiddleAction,
      MainAction.endAction: _onEndAction,
    },
  );
}

MainState _onMiddleAction(MainState state, Action action) {
  println('Reducer: middleAction');
  return state;
}

MainState _onEndAction(MainState state, Action action) {
  println('Reducer: endAction');
  return state;
}

To reproduce dispatch begin action from view.

dispatch(MainActionCreator.onStartAction());

Expected behavior All effects should be called. All reducers should be called.

Additional context

  1. The version of fish-redux which you are using.
  2. The information from flutter doctor.
achernoprudov commented 4 years ago

I found in sources problem place:

/// lib/src/redux_component/helper.dart

Effect<T> combineEffects<T>(Map<Object, SubEffect<T>> map) =>
    ....
    if (subEffect != null) {
        return subEffect.call(action, ctx) ?? _SUB_EFFECT_RETURN_NULL;
    }
    ....
    return null;
};

If subEffect doesn't return value the combineEffect returns _SUB_EFFECT_RETURN_NULL. And after this in createDispatch condition ignores to call reducer:

Dispatch createDispatch<T>(Dispatch onEffect, Dispatch next, Context<T> ctx) =>
    (Action action) {
      final Object result = onEffect?.call(action);
      if (result == null || result == false) { //  <<< here result equals to _SUB_EFFECT_RETURN_NULL
        next(action);
      }

      return result == _SUB_EFFECT_RETURN_NULL ? null : result;
    };

On this basis we can use workaround and make effect to return false. Then everything works fine but if we has more complicated logic where effect returns some value then this wont work anymore. Second point that returning false its not obvious for me and other developers so it has to be fixed anyway.

achernoprudov commented 4 years ago

It can be fixed in two ways:

  1. Remove returning _SUB_EFFECT_RETURN_NULL from combineEffects.

    Effect<T> combineEffects<T>(Map<Object, SubEffect<T>> map) =>
    ....
    if (subEffect != null) {
        return subEffect.call(action, ctx);
    }
    ....
    return null;
    };
  2. Add another OR condition to if in createDispatch:

    Dispatch createDispatch<T>(Dispatch onEffect, Dispatch next, Context<T> ctx) =>
    (Action action) {
      final Object result = onEffect?.call(action);
      if (result == null || result == false || result == _SUB_EFFECT_RETURN_NULL) {
        next(action);
      }
    
      return result == _SUB_EFFECT_RETURN_NULL ? null : result;
    };

I am ready to make pull request with fix but I don't know what I can break with this fix.

henry-hz commented 3 years ago

@achernoprudov good catch!