felangel / bloc

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

on<TEvent> - possibility to override "one for all" event handler with concrete one #2855

Closed liri2006 closed 3 years ago

liri2006 commented 3 years ago

Hi @felangel, I'm trying to migrate to the new way of handling of events. There is bit of "magic" added around the bloc in our project, so that event handling logic is written in separate event classes instead of single bloc class. And all of that was handled in mapEventToState in the base class, so, naturally code was changed to do the same job in a single on handler:

 AppBloc(TState initialState) : super(initialState) {
    on<TEvent>(_onEvent, transformer: sequential());
  }

And that works as expected with almost no refactoring to the rest of the app. The only issue we have is that with deletion of transformEvents the ability to debounce some events would be lost - I've tried to register handler for concrete event type, but it was not called, main handler was called instead.

   SearchBloc({
    required this.personalizationRepository,
    required this.searchRepository,
  }) : super(SearchState.initial()) {
    on<Search_SearchEvent>(
      (Search_SearchEvent event, Emitter<SearchState> emit) {
        Log.e(tag, event.runtimeType.toString());
      },
      transformer: debounce(const Duration(milliseconds: 300)),
    );
  }

Is there a way to have central event handling method in the base class and ability to add custom handlers with transformers for some events?

felangel commented 3 years ago

Hi @liri2006 👋 Thanks for opening an issue!

Can you please provide a link to a minimal reproduction sample that illustrates the issue you're facing? Thanks! 🙏

liri2006 commented 3 years ago

@felangel Here is a showcase of the issue. While preparing the example I found out that I was a bit off while describing the issue - both debounced AND central event handlers are being called after event is dispatched, which result in counter being incremented during every + button tap and then additionally one more time after debounce time.

import 'dart:async';

import 'package:bloc/bloc.dart';
import 'package:bloc_concurrency/bloc_concurrency.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:rxdart/rxdart.dart';

void main() {
  Bloc.observer = CounterObserver();
  runApp(const CounterApp());
}

class CounterApp extends MaterialApp {
  const CounterApp({Key? key})
      : super(
          key: key,
          home: const CounterPage(),
        );
}

class CounterPage extends StatelessWidget {
  const CounterPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      create: (_) => CounterBloc(),
      child: CounterView(),
    );
  }
}

// bloc implementation
class CounterBloc extends AppBloc<CounterEvent, CounterState> {
  CounterBloc() : super(CounterState(count: 0)) {
    on<IncrementEvent>(
      (IncrementEvent event, Emitter<CounterState> emit) {
        print('On debounced event!');
        emit(state.copyWith(count: state.count + 1));
      },
      transformer: debounce(const Duration(milliseconds: 800)),
    );
  }

  EventTransformer<T> debounce<T>(Duration duration) {
    return (events, mapper) => events.debounceTime(duration).flatMap(mapper);
  }

  void increment() => add(IncrementEvent());
  void decrement() => add(DecrementEvent());
}

class CounterState with AppBlocState<CounterState> {
  CounterState({required this.count});

  final int count;

  CounterState copyWith({int? count}) {
    return CounterState(
      count: count ?? this.count,
    );
  }
}

abstract class CounterEvent extends AppBlocEvent<CounterState, CounterBloc> {}

class IncrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count + 1);
  }
}

class DecrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count - 1);
  }
}

// Abstract classes

abstract class AppBloc<TEvent extends AppBlocEvent, TState extends AppBlocState> extends Bloc<TEvent, TState> {
  AppBloc(initialState) : super(initialState) {
    on<TEvent>(_onEvent, transformer: sequential());
  }

  FutureOr<void> _onEvent(TEvent event, Emitter<TState> emit) async {
    var frozenState = state;
    try {
      final errorHandlingTransform = StreamTransformer<TState, TState>.fromHandlers(
        handleError: (Object error, StackTrace stackTrace, EventSink<TState> sink) {
          sink.add(frozenState);
        },
      );

      final statesStream = event.apply(this).transform(errorHandlingTransform);
      await statesStream.listen((TState event) {
        emit(event);
      }).asFuture();
    } catch (e) {
      emit(frozenState);
    }
  }
}

abstract class AppBlocEvent<TState extends AppBlocState, TBloc extends Bloc> {
  Stream<TState> apply(TBloc bloc);

  @override
  String toString() => '${runtimeType.toString()} | timestamp: ${DateTime.now().toIso8601String()}';
}

abstract class AppBlocState<TState> {}

// view
class CounterView extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;
    return Scaffold(
      appBar: AppBar(title: const Text('Counter')),
      body: Center(
        child: BlocBuilder<CounterBloc, CounterState>(
          builder: (context, state) {
            return Text('${state.count}', style: textTheme.headline2);
          },
        ),
      ),
      floatingActionButton: Column(
        mainAxisAlignment: MainAxisAlignment.end,
        crossAxisAlignment: CrossAxisAlignment.end,
        children: <Widget>[
          FloatingActionButton(
            key: const Key('counterView_increment_floatingActionButton'),
            child: const Icon(Icons.add),
            onPressed: () => context.read<CounterBloc>().increment(),
          ),
          const SizedBox(height: 8),
          FloatingActionButton(
            key: const Key('counterView_decrement_floatingActionButton'),
            child: const Icon(Icons.remove),
            onPressed: () => context.read<CounterBloc>().decrement(),
          ),
        ],
      ),
    );
  }
}

class CounterObserver extends BlocObserver {
  @override
  void onEvent(Bloc bloc, Object? event) {
    super.onEvent(bloc, event);

    print(event.toString());
  }
}
felangel commented 3 years ago

@liri2006 thanks for the reproduction sample! Are you able to provide an equivalent sample using mapEventToState (the way you were previously doing things)? Also, can you explain a bit more why you are using this approach? Ideally events should not contain any logic (that's the responsibility of the bloc). Also, uncaught exceptions are automatically handled by bloc so you shouldn't need to write a custom error handling stream transformer. I highly recommend following the documented approach but if you can provide a working sample of the above code using the old mapEventToState Api it would be much easier for me to put together a migrated version of the legacy implementation, thanks!

liri2006 commented 3 years ago

@felangel We are using flutter_bloc since 0.14.0 version and library has evolved significantly in terms of trimming down the boilerplate. This approach was implemented to allow us to write less code and to allow us to divide logic into separate files (to "add" to codebase rather than "modify" it whenever there is a new feature we want to implement). If everything is done inside the bloc file, than pretty quickly it becomes bloated, it becomes harder to read, extend and maintain it. Event files are more event handlers than events (native naming just stuck with us :). It is much easier to locate piece of logic when it is structured like this: image rather being written inside of a single file.

As to error handling - I've attached a trimmed down version of the implementation, we have some other code (except yielding pre-event state on error) called during error, like making sure that modal loader is switched off, snack bar is hidden, etc (just some quality of life helpers). Also this was implemented over 2 years ago and not revisited ever since, so we just have missed blocs' error handling improvements :)

Here is a sample using mapEventToState.

import 'dart:async';

import 'package:bloc/bloc.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:rxdart/rxdart.dart';

void main() {
  Bloc.observer = CounterObserver();
  runApp(const CounterApp());
}

class CounterApp extends MaterialApp {
  const CounterApp({Key? key})
      : super(
          key: key,
          home: const CounterPage(),
        );
}

class CounterPage extends StatelessWidget {
  const CounterPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      create: (_) => CounterBloc(),
      child: CounterView(),
    );
  }
}

// bloc implementation
class CounterBloc extends AppBloc<CounterEvent, CounterState> {
  CounterBloc() : super(CounterState(count: 0));

  @override
  Stream<Transition<CounterEvent, CounterState>> transformEvents(
    Stream<CounterEvent> events,
    TransitionFunction<CounterEvent, CounterState> next,
  ) {
    final nonDebounceStream = events.where((CounterEvent event) {
      return event is! IncrementEvent;
    });

    final followDebounceStream = events.where((CounterEvent event) {
      return event is IncrementEvent;
    }).debounceTime(const Duration(
      milliseconds: 300,
    ));

    return super.transformEvents(
        nonDebounceStream.mergeWith(<Stream<CounterEvent>>[
          followDebounceStream,
        ]),
        next);
  }

  void increment() => add(IncrementEvent());
  void decrement() => add(DecrementEvent());
}

class CounterState with AppBlocState<CounterState> {
  CounterState({required this.count});

  final int count;

  CounterState copyWith({int? count}) {
    return CounterState(
      count: count ?? this.count,
    );
  }
}

abstract class CounterEvent extends AppBlocEvent<CounterState, CounterBloc> {}

class IncrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count + 1);
  }
}

class DecrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count - 1);
  }
}

// Abstract classes

abstract class AppBloc<TEvent extends AppBlocEvent, TState extends AppBlocState> extends Bloc<TEvent, TState> {
  AppBloc(initialState) : super(initialState);

  @override
  Stream<TState> mapEventToState(TEvent event) async* {
    var frozenState = state;
    try {
      final errorHandlingTransform = StreamTransformer<TState, TState>.fromHandlers(
        handleError: (Object error, StackTrace stackTrace, EventSink<TState> sink) {
          sink.add(frozenState);
        },
      );

      yield* event.apply(this).transform(errorHandlingTransform);
    } catch (e) {
      yield frozenState;
    }
  }
}

abstract class AppBlocEvent<TState extends AppBlocState, TBloc extends Bloc> {
  Stream<TState> apply(TBloc bloc);

  @override
  String toString() => '${runtimeType.toString()} | timestamp: ${DateTime.now().toIso8601String()}';
}

abstract class AppBlocState<TState> {}

// view
class CounterView extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;
    return Scaffold(
      appBar: AppBar(title: const Text('Counter')),
      body: Center(
        child: BlocBuilder<CounterBloc, CounterState>(
          builder: (context, state) {
            return Text('${state.count}', style: textTheme.headline2);
          },
        ),
      ),
      floatingActionButton: Column(
        mainAxisAlignment: MainAxisAlignment.end,
        crossAxisAlignment: CrossAxisAlignment.end,
        children: <Widget>[
          FloatingActionButton(
            key: const Key('counterView_increment_floatingActionButton'),
            child: const Icon(Icons.add),
            onPressed: () => context.read<CounterBloc>().increment(),
          ),
          const SizedBox(height: 8),
          FloatingActionButton(
            key: const Key('counterView_decrement_floatingActionButton'),
            child: const Icon(Icons.remove),
            onPressed: () => context.read<CounterBloc>().decrement(),
          ),
        ],
      ),
    );
  }
}

class CounterObserver extends BlocObserver {
  @override
  void onEvent(Bloc bloc, Object? event) {
    super.onEvent(bloc, event);

    print(event.toString());
  }
}
liri2006 commented 3 years ago

I was able to workaround the issue of double event handling by storing the concrete type of the debounced event and discard it in central event handler. But it would be great to see something like that supported natively, especially given the fact that central event handler is one of the proposed ways of restoring previous behavior by the documentation. And doing that would not allow to apply transformers for just some of the events.

import 'dart:async';

import 'package:bloc/bloc.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:rxdart/rxdart.dart';

void main() {
  Bloc.observer = CounterObserver();
  runApp(const CounterApp());
}

class CounterApp extends MaterialApp {
  const CounterApp({Key? key})
      : super(
          key: key,
          home: const CounterPage(),
        );
}

class CounterPage extends StatelessWidget {
  const CounterPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      create: (_) => CounterBloc(),
      child: CounterView(),
    );
  }
}

// bloc implementation
class CounterBloc extends AppBloc<CounterEvent, CounterState> {
  CounterBloc() : super(CounterState(count: 0)) {
    registerDebounceable<IncrementEvent>(const Duration(milliseconds: 800));
  }

  void increment() => add(IncrementEvent());
  void decrement() => add(DecrementEvent());
}

class CounterState with AppBlocState<CounterState> {
  CounterState({required this.count});

  final int count;

  CounterState copyWith({int? count}) {
    return CounterState(
      count: count ?? this.count,
    );
  }
}

abstract class CounterEvent extends AppBlocEvent<CounterState, CounterBloc> {}

class IncrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count + 1);
  }
}

class DecrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count - 1);
  }
}

// Abstract classes

abstract class AppBloc<TEvent extends AppBlocEvent, TState extends AppBlocState> extends Bloc<TEvent, TState> {
  final List<Type> _customHandledEvents = <Type>[];

  AppBloc(initialState) : super(initialState) {
    on<TEvent>(_onEventCentral);
  }

  FutureOr<void> _onEventCentral(TEvent event, Emitter<TState> emit) async {
    if (_customHandledEvents.any((Type type) => type == event.runtimeType)) {
      print('Skipped central handler for ${event.runtimeType}');
      return;
    }

    return _onEvent(event, emit);
  }

  FutureOr<void> _onEvent(TEvent event, Emitter<TState> emit) async {
    var frozenState = state;
    try {
      final errorHandlingTransform = StreamTransformer<TState, TState>.fromHandlers(
        handleError: (Object error, StackTrace stackTrace, EventSink<TState> sink) {
          sink.add(frozenState);
        },
      );

      final statesStream = event.apply(this).transform(errorHandlingTransform);
      await statesStream.listen((TState event) {
        emit(event);
      }).asFuture();
    } catch (e) {
      emit(frozenState);
    }
  }

  void registerDebounceable<E extends TEvent>(Duration duration) {
    _customHandledEvents.add(E);
    on<E>(_onEventDebounce, transformer: debounce(duration));
  }

  FutureOr<void> _onEventDebounce(TEvent event, Emitter<TState> emit) async {
    print('Debounced event ${event.runtimeType}');

    return _onEvent(event, emit);
  }

  EventTransformer<T> debounce<T>(Duration duration) {
    return (events, mapper) => events.debounceTime(duration).asyncExpand(mapper);
  }
}

abstract class AppBlocEvent<TState extends AppBlocState, TBloc extends Bloc> {
  Stream<TState> apply(TBloc bloc);

  @override
  String toString() => '${runtimeType.toString()} | timestamp: ${DateTime.now().toIso8601String()}';
}

abstract class AppBlocState<TState> {}

// view
class CounterView extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;
    return Scaffold(
      appBar: AppBar(title: const Text('Counter')),
      body: Center(
        child: BlocBuilder<CounterBloc, CounterState>(
          builder: (context, state) {
            return Text('${state.count}', style: textTheme.headline2);
          },
        ),
      ),
      floatingActionButton: Column(
        mainAxisAlignment: MainAxisAlignment.end,
        crossAxisAlignment: CrossAxisAlignment.end,
        children: <Widget>[
          FloatingActionButton(
            key: const Key('counterView_increment_floatingActionButton'),
            child: const Icon(Icons.add),
            onPressed: () => context.read<CounterBloc>().increment(),
          ),
          const SizedBox(height: 8),
          FloatingActionButton(
            key: const Key('counterView_decrement_floatingActionButton'),
            child: const Icon(Icons.remove),
            onPressed: () => context.read<CounterBloc>().decrement(),
          ),
        ],
      ),
    );
  }
}

class CounterObserver extends BlocObserver {
  @override
  void onEvent(Bloc bloc, Object? event) {
    super.onEvent(bloc, event);

    print(event.toString());
  }
}
felangel commented 3 years ago

@liri2006 thanks for the additional context. I migrated your mapEventToState approach to the new on<E> API:

import 'dart:async';

import 'package:bloc/bloc.dart';
import 'package:bloc_concurrency/bloc_concurrency.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:rxdart/rxdart.dart';

void main() {
  Bloc.observer = CounterObserver();
  runApp(const CounterApp());
}

class CounterApp extends MaterialApp {
  const CounterApp({Key? key})
      : super(
          key: key,
          home: const CounterPage(),
        );
}

class CounterPage extends StatelessWidget {
  const CounterPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return BlocProvider(
      create: (_) => CounterBloc(),
      child: CounterView(),
    );
  }
}

// bloc implementation
class CounterBloc extends AppBloc<CounterEvent, CounterState> {
  CounterBloc() : super(CounterState(count: 0));

  @override
  EventTransformer<CounterEvent> eventTransformer() {
    return (events, mapper) {
      final nonDebounceStream = events.where((CounterEvent event) {
        return event is! IncrementEvent;
      });

      final followDebounceStream = events.where((CounterEvent event) {
        return event is IncrementEvent;
      }).debounceTime(const Duration(milliseconds: 300));

      return Rx.merge([nonDebounceStream, followDebounceStream])
          .asyncExpand(mapper);
    };
  }

  void increment() => add(IncrementEvent());
  void decrement() => add(DecrementEvent());
}

class CounterState with AppBlocState<CounterState> {
  CounterState({required this.count});

  final int count;

  CounterState copyWith({int? count}) {
    return CounterState(
      count: count ?? this.count,
    );
  }
}

abstract class CounterEvent extends AppBlocEvent<CounterState, CounterBloc> {}

class IncrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count + 1);
  }
}

class DecrementEvent extends CounterEvent {
  @override
  Stream<CounterState> apply(CounterBloc bloc) async* {
    yield bloc.state.copyWith(count: bloc.state.count - 1);
  }
}

// Abstract classes

abstract class AppBloc<TEvent extends AppBlocEvent, TState extends AppBlocState>
    extends Bloc<TEvent, TState> {
  AppBloc(initialState) : super(initialState) {
    on<TEvent>((event, emit) {
      return emit.forEach(
        event.apply(this).cast<TState>(),
        onData: (TState state) => state,
      );
    }, transformer: eventTransformer());
  }

  EventTransformer<TEvent> eventTransformer() => sequential<TEvent>();
}

abstract class AppBlocEvent<TState extends AppBlocState, TBloc extends Bloc> {
  Stream<TState> apply(TBloc bloc);

  @override
  String toString() =>
      '${runtimeType.toString()} | timestamp: ${DateTime.now().toIso8601String()}';
}

abstract class AppBlocState<TState> {}

// view
class CounterView extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;
    return Scaffold(
      appBar: AppBar(title: const Text('Counter')),
      body: Center(
        child: BlocBuilder<CounterBloc, CounterState>(
          builder: (context, state) {
            return Text('${state.count}', style: textTheme.headline2);
          },
        ),
      ),
      floatingActionButton: Column(
        mainAxisAlignment: MainAxisAlignment.end,
        crossAxisAlignment: CrossAxisAlignment.end,
        children: <Widget>[
          FloatingActionButton(
            key: const Key('counterView_increment_floatingActionButton'),
            child: const Icon(Icons.add),
            onPressed: () => context.read<CounterBloc>().increment(),
          ),
          const SizedBox(height: 8),
          FloatingActionButton(
            key: const Key('counterView_decrement_floatingActionButton'),
            child: const Icon(Icons.remove),
            onPressed: () => context.read<CounterBloc>().decrement(),
          ),
        ],
      ),
    );
  }
}

class CounterObserver extends BlocObserver {
  @override
  void onEvent(Bloc bloc, Object? event) {
    super.onEvent(bloc, event);

    print(event.toString());
  }
}

Again I highly encourage you to reconsider this approach because your events should not contain the business logic. If you prefer to split things out in order to be more organized you can just register multiple handlers and define the event handlers in separate files:

MyBloc() : super(MyState()) {
  on<EventA>(_onEventA);
  on<EventB>(_onEventB);
}

Where _onEventA and _onEventB are defined in event_a_handler.dart and event_b_handler.dart respectively:

/// event_a_handler.dart
FutureOr<void> _onEventA(EventA event, Emitter<MyState> emit) {...}
/// event_b_handler.dart
FutureOr<void> _onEventB(EventB event, Emitter<MyState> emit) {...}

Also, as I mentioned earlier the additional error handling stream transformer is unnecessary. Hope that helps 👍

felangel commented 3 years ago

Closing for now since there doesn't appear to be any additional actions needed. Feel free to comment with any follow up questions and I'm happy to continue the conversation 👍

liri2006 commented 3 years ago

Thank you very much for the sample!

Regarding the events I completely agree with you regarding the separation. As the mater of fact we do have similar Event/Handler architecture on the server side - event is just a data object and handler contains the logic. But on the server we have a reflection, so that the handler can be automatically resolved by event type with no need to manually register it.

In the everything just comes to cutting down the boilerplate and making the development & maintenance easier. It is just a compromise that we are able to accept - decoupling event from handling would not bring that much to the table in terms of readability/maintainability, but will introduce the need to create additional class for every event and the need to register handler manually (which can be easily forgotten). And we have hundreds of events, so it does not scale well :) This can be probably solved by some code generation, but that will introduce some other issues.

Anyways, something to think about. Thank you again for the help and your dedication to this lib!

prajwal27 commented 3 years ago

@liri2006 even we have followed a similar architecture. So, it would be great to discuss this architecture somewhere else (discord/slack). Please let me know.