felangel / bloc

A predictable state management library that helps implement the BLoC design pattern
https://bloclibrary.dev
MIT License
11.84k stars 3.4k forks source link

onTransition not called if state extends equatable #778

Closed evmartinelli closed 4 years ago

evmartinelli commented 4 years ago

Describe the bug I was implementing a variation of the weather example and the onTransition was never get called twice.

To Reproduce

abstract class ExchangeState extends Equatable {
  const ExchangeState();

  @override
  List<Object> get props => [];
}

class ExchangeLoading extends ExchangeState {}
class ExchangeError extends ExchangeState {}

class ExchangeLoaded extends ExchangeState {
  const ExchangeLoaded({@required this.exchange}) : assert (exchange != null);
  final Exchange exchange;

  @override
  List<Object> get props => [exchange];

}

with equatable flutter: FetchExchange flutter: Transition { currentState: Instance of 'ExchangeLoading', event: FetchExchange, nextState: Instance of 'ExchangeLoaded' } flutter: RefreshExchange (no transition called)

without equatable flutter: FetchExchange flutter: Transition { currentState: Instance of 'ExchangeLoading', event: FetchExchange, nextState: Instance of 'ExchangeLoaded' } flutter: RefreshExchange flutter: Transition { currentState: Instance of 'ExchangeLoaded', event: RefreshExchange, nextState: Instance of 'ExchangeLoaded' } flutter: RefreshExchange flutter: Transition { currentState: Instance of 'ExchangeLoaded', event: RefreshExchange, nextState: Instance of 'ExchangeLoaded' } flutter: RefreshExchange

Logs [✓] Flutter (Channel stable, v1.12.13+hotfix.5, on Mac OS X 10.15.2 19C57, locale en-BR) • Flutter version 1.12.13+hotfix.5 at /Users/evandrom/Study/flutter • Framework revision 27321ebbad (5 weeks ago), 2019-12-10 18:15:01 -0800 • Engine revision 2994f7e1e6 • Dart version 2.7.0

[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3) • Android SDK at /Users/evandrom/Library/Android/sdk • Android NDK location not configured (optional; useful for native profiling support) • Platform android-28, build-tools 28.0.3 • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405) • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.3) • Xcode at /Applications/Xcode.app/Contents/Developer • Xcode 11.3, Build version 11C29 • CocoaPods version 1.8.4

[!] Android Studio (version 3.5) • Android Studio at /Applications/Android Studio.app/Contents ✗ Flutter plugin not installed; this adds Flutter specific functionality. ✗ Dart plugin not installed; this adds Dart specific functionality. • Java version OpenJDK Runtime Environment (build 1.8.0_202-release-1483-b49-5587405)

[!] IntelliJ IDEA Community Edition (version 2019.1.3) • IntelliJ at /Applications/IntelliJ IDEA CE.app ✗ Flutter plugin not installed; this adds Flutter specific functionality. ✗ Dart plugin not installed; this adds Dart specific functionality. • For information about installing plugins, see https://flutter.dev/intellij-setup/#installing-the-plugins

[✓] VS Code (version 1.41.1) • VS Code at /Applications/Visual Studio Code.app/Contents • Flutter extension version 3.7.1

[✓] Connected device (1 available) • iPhone 8 • BD3BE923-99F3-4B7C-A1DB-E9801B7A07CD • ios • com.apple.CoreSimulator.SimRuntime.iOS-13-3 (simulator)

felangel commented 4 years ago

Hi @evandrom 👋 Thanks for opening an issue!

This is working as expected.

Blocs will ignore duplicate states. If a Bloc yields State nextState where state == nextState, then no transition will occur and no change will be made to the Stream. (source)

Equatable makes two state instances equal if their values/props are equal. As a result, if you extend Equatable and emit back to back states with the same values/props, no transition will occur.

Hope that helps 👍

pinkeshdarji commented 4 years ago

@felangel I am facing the same issue. Could you please suggest any solution?

theDarkBoffin commented 4 years ago

@pinkeshdarji, As @felangel has already said,

Equatable makes two state instances equal if their values/props are equal. As a result, if you extend Equatable and emit back to back states with the same values/props, no transition will occur.

If you're still not sure why you're facing this issue, please post a code snippet on how you're using mapEventToState or a repo link so that you could be assisted better. 👍

pinkeshdarji commented 4 years ago

@felangel @theDarkBoffin Thanks for the quick reply. Unfortunately, I won't be able to post code here for now. But I can try to explain my use case here.

1) I want to emit the same state until some condition is matched. Like if the download is less than 100% then I emit same state with download progress. 2) I want to have unit test over this.

Now the state extends equitable only to write unit test case on it. If I make state not extend equitable then test case will always fail and If I make it extend then the same state won't get emitted.

Please help.

theDarkBoffin commented 4 years ago

@pinkeshdarji, if you could just write a dummy code snippet as to what you're trying to achieve, it'll help to see why you're facing the problem. However, with your explanation, I can give some pointers as to why this is happening.

  1. When you extend Equatable, you need to override the props function and return an array of props which you think is helpful to differentiate two objects of the same state. For example, if StateA has props color and size, props function should look like

    @override
    List<Object> get props => [color, size];

    If you miss out color and return only [size], state transitions from old StateA to new StateA will happen only when size is changed. color changes are not respected.

  2. If you're having an object, say downloadedPosts(list of posts) as your prop, following is the wrong way to expect a transition from old StateA to new StateA

    if(event is DownloadedNewPost) {
    final currentState = state as StateA;
    currentState.downloadedPosts.add(event.newPost);
    final nextState = StateA(currentState.downloadedPosts);
    yield nextState;
    }

    What's happening here is, currentState's downloadedPosts is being mutated, and then passed to nextState object. Since nextState and currentState refer to the same downloadedPosts, Bloc thinks that nothing has changed since Equatable says that both states are same. What you must do in such a case, is to create a new list from existing list, and then append to that list, which is what the below code snipped does.

    if(event is DownloadedNewPost) {
    final currentState = state as StateA;
    final nextState = StateA(currentState.downloadedPosts + [event.newPost]);
    yield nextState;
    }

If you're facing issues with state transitions, the only reason might be that you're not properly overriding props, or not yielding new state properly. I hope this helps.

pinkeshdarji commented 4 years ago

Thanks, @theDarkBoffin Your suggestions worked. I was not overriding all props. After putting all properties inside [] fixed the issue.

mbfakourii commented 4 years ago

Hi @evandrom Thanks for opening an issue!

This is working as expected.

Blocs will ignore duplicate states. If a Bloc yields State nextState where state == nextState, then no transition will occur and no change will be made to the Stream. (source)

Equatable makes two state instances equal if their values/props are equal. As a result, if you extend Equatable and emit back to back states with the same values/props, no transition will occur.

Hope that helps

What if we need a duplicate value in Listner?

rizirf-connect commented 4 years ago

Hey @felangel My bloc builder does not triggered on second time here is my bloc

import 'dart:async';

import 'package:bloc/bloc.dart';
import 'package:brooklyn_bites/Infrastructure/core/app_database.dart';
import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:injectable/injectable.dart';

part 'manage_cart_state.dart';
part 'manage_cart_event.dart';
part 'manage_cart_bloc.freezed.dart';

@injectable
class ManageCartBloc extends Bloc<ManageCartEvent, ManageCartState> {
  ManageCartBloc(this.db) : super(ManageCartState.initial());

  final AppDatabase db;

  @override
  Stream<ManageCartState> mapEventToState(ManageCartEvent gEvent) async* {
    if (gEvent is _AddItemToCart) {
      yield state.copyWith(
        cartItems: List.from(
          state.cartItems
            ..add(
              CartItems(
                productId: gEvent.product.id,
                quantity: gEvent.quantity,
              ),
            ),
        ),
      );
    }

    if (gEvent is _RemoveItemFromCart) {
      yield state.copyWith(
          cartItems: List.from(state.cartItems
            ..remove(CartItems(
                productId: gEvent.product.id, quantity: gEvent.quantity))));
    }

    if (gEvent is _UpdateCartItem) {
      yield state.copyWith(
        cartItems: List.from(state.cartItems)
          ..where((element) => element.productId == gEvent.productId)
              .first
              .quantity += 1,
      );
    }
  }
}
felangel commented 4 years ago

Hi @rizirf-connect 👋 Does CartItems extend Equatable?

rizirf-connect commented 4 years ago

No My cart items does not extend ewuatable its just a simple class

felangel commented 4 years ago

@rizirf-connect if your state extends Equatable then all properties need to also extend Equatable

rizirf-connect commented 4 years ago

Sir Im using bloc with freezed package

rizirf-connect commented 4 years ago

https://github.com/rizirf-connect/brooklyn_bites/tree/master/lib/application/manage_cart_bloc Thi is my repo

felangel commented 4 years ago

Yeah same thing applies with freezed. Your state properties need to also override == and hashCode.

rizirf-connect commented 4 years ago

You mean I have to use Freezed class of Cart Items instead of simple class

felangel commented 4 years ago

@rizirf-connect yes or you can override == and hashCode a different way.

rizirf-connect commented 4 years ago

Thankyou so much Sir @felangel you're the real bOss I'll let you know if this solves my problem thumbs up for bloc 👍

rizirf-connect commented 4 years ago

Hey @felangel thankyou so much for your response now its working properly 👍