Remove RxDart dependency from bloc package #821

mozhaiskyi · 2020-01-30T19:09:14Z

Status

READY

Breaking Changes

NO

Description

BloC is architecture library which used as the foundation of the application. In my opinion it should be independent of any other libraries like RxDart.

Rx code was used only in two places inside bloc package.

bloc.dart (PublishSubject, BehaviorSubject replaced with two StreamController. Also added variable for storing current state which called _currentState)
complex_bloc.dart (File is test helper. It still using RxDart because package is enable as a test dependency)

packages/bloc/lib/src/bloc.dart

felangel

Just had a few minor comments but overall awesome PR! Given the recent changes to rxdart (migration to extensions on Stream) I think this makes a lot of sense. Thanks so much for opening this 💯

tenhobi · 2020-01-31T10:23:38Z

I am not sure atm, but using BehaviorSubject you are given the last emitted state (from cache) when you subscribe (was _stateSubject.listen) even after some changes to the bloc have been made (emitting a new state). What part of the code does this now? Not sure it's part of the tests to cover this issue.

BehaviorSubject - A broadcast StreamController that caches the latest added value or error. When a new listener subscribes to the Stream, the latest value or error will be emitted to the listener. Furthermore, you can synchronously read the last emitted value.

What I mean:

subscribe to BlocA with X
add an event1
that emits a state1
subscribe to BlocA with Y
does the Y gets state1 now ??

jorgecoca · 2020-01-31T14:27:48Z

I am not sure atm, but using BehaviorSubject you are given the last emitted state (from cache) when you subscribe (was _stateSubject.listen) even after some changes to the bloc have been made (emitting a new state). What part of the code does this now? Not sure it's part of the tests to cover this issue.

BehaviorSubject - A broadcast StreamController that caches the latest added value or error. When a new listener subscribes to the Stream, the latest value or error will be emitted to the listener. Furthermore, you can synchronously read the last emitted value.

What I mean:

subscribe to BlocA with X

add an event1

that emits a state1

subscribe to BlocA with Y

does the Y gets state1 now ??

@felangel is there a test case for this? If not, I think that would be a good addition ;)

packages/bloc/pubspec.yaml

packages/bloc/lib/src/bloc.dart

mozhaiskyi · 2020-01-31T21:26:18Z

PR contains problems. Changed status to IN DEVELOPMENT. I will try to find better solution.

@tenhobi you are right. Solution with StreamController does not include previous value when new subscription established. It is critical (missing initial state when widget was rebuilded). Also _states should be a broadcast to allow multi subscriptions.

I propose to add test to cover this issue in separate PR. It will be very useful.

subscribe to BlocA with X

add an event1

that emits a state1

subscribe to BlocA with Y

does the Y gets state1 now ??

@tenhobi thank you. Good catch 👍

mozhaiskyi · 2020-02-01T20:09:20Z

Implemented StateSubject (please propose different name if it is necessary). It is lightweight analog of BehaviorSubject. This approach fixed issues related to last item emit. Also BLoC support several listeners as well.

Added tests which cover StateSubject logic such as emits, seed item emit, errors, null-values. Test cases with several listeners are included.

FYI @felangel @tenhobi

felangel · 2020-02-01T21:19:14Z

@mmozhaiskyi thanks for your efforts! I have a few concerns about this approach and would love to hear your thoughts:

StateSubject should not be publicly exposed in my opinion as it's just an implementation detail.
I don't like the idea of copying the BehaviorSubject implementation straight from rxdart just to remove the dependency. It's not a trivial implementation (especially in it's current form) and I think it could be further simplified because it will be used in a very specific context.
We now run the risk of having to maintain the same implementation in multiple places which isn't beneficial for the community and doesn't feel right to me.

Thoughts? (cc @brianegan)

felangel · 2020-02-02T02:39:36Z

Was just messing around on my own branch and managed to simplify the implementation a bit more (I think). What do y'all think?

mozhaiskyi · 2020-02-02T10:06:29Z

@felangel Thank you for your feedback.

Your proposal is awesome and useful. Really, we don’t need full implementation of BehaviourSubject from Rx because bloc context is very specific. I simplified solution as it possible (see last commit).

Changes:

current approach uses _StartWithStreamTransformer only.
logic of last value emit moved into bloc (StateSubject abstraction was removed).

packages/bloc/lib/src/bloc.dart

Co-Authored-By: Felix Angelov <felangelov@gmail.com>

felangel

LGTM 💯
Thanks for your patience and for incorporating all of the feedback so quickly 👍

@jorgecoca, @brianegan thoughts? It ended up being almost identical to Brian's PR which was by far the most simple approach.

packages/bloc/lib/src/bloc.dart

brianegan · 2020-02-02T18:57:50Z

Wait, my bad -- was coding too quickly. My implementation has a big problem: You can't listen to the state stream twice! This test would fail with this current implementation. I would go back to using a StreamController<State>.broadcast() for the stateStream. It's also probably a good idea to add a test case that verifies the Stream can be listened to more than once.

      test('close does not emit new states over the state stream', () {
        final expectedStates = [equals(''), emitsDone];

        expectLater(
          simpleBloc,
          emitsInOrder(expectedStates),
        );

        expectLater(
          simpleBloc,
          emitsInOrder(expectedStates),
        );

        simpleBloc.close();
      });

This gets a bit hairy: I changed that state StreamController from a broadcast to a normal StreamController to prevent any errors using an async* for the startWith implementation, which were displayed by @mmozhaiskyi in his test code above. Of course, that means you can no longer listen to the Stream more than once 🤦‍♂

The cause of that issue in the tests? Events were added to the StreamController before the async* function could start listening (as the function is run asynchronously).

The Bloc class will not have that same problem, because events are not synchronously added to the stateController, but rather added asynchronously after being transformed by transformEvent and transformState. Therefore, the async* function has time to execute and start listening to the broadcast state Stream before any events would be sent. That's why this test actually works fine using the async* function for startsWith + a broadcast Stream on the Bloc class:

      test('should map single event to correct state', () {
        final expectedStates = ['', 'data'];

        expectLater(
          simpleBloc,
          emitsInOrder(expectedStates),
        );

        expectLater(
          simpleBloc,
          emitsInOrder(expectedStates),
        );

        expectLater(
          simpleBloc,
          emitsInOrder(expectedStates),
        );

        simpleBloc.add('event');
      });

brianegan · 2020-02-02T19:02:45Z

Updated my branch with those changes!

brianegan · 2020-02-02T19:49:13Z

packages/bloc/lib/src/bloc.dart

@@ -10,7 +10,7 @@ import '../bloc.dart';
 /// {@endtemplate}
 abstract class Bloc<Event, State> extends Stream<State> implements Sink<Event> {
  final _eventController = StreamController<Event>.broadcast();
-  final _stateController = StreamController<State>();
+  final _stateController = StreamController<State>.broadcast();


Looks good! I'd just make sure to add the test to catch this bug as well.

felangel · 2020-02-02T22:53:56Z

Going to merge and will apply the testing updates right after. Thanks everyone!

codecov · 2020-02-02T22:59:37Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8d31770). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff           @@
##             master   #821   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?     16           
  Lines             ?    247           
  Branches          ?      0           
=======================================
  Hits              ?    247           
  Misses            ?      0           
  Partials          ?      0

Impacted Files	Coverage Δ
packages/bloc/lib/src/bloc.dart	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d31770...a2fca17. Read the comment docs.

sla-000 · 2022-08-19T08:00:15Z

Hi @felangel @mozhaiskyi So if I understand it right, though BehaviorSubject was removed from bloc/cubit but some other mechanism was implemented to keep it work as it was with BehaviorSubject?
I added listener to the cubit example from repo (see below) and I can see that listener does not get last value from cubit right after it connects to states stream. Listener only gets a new state if I call increment() after connection.
Is it supposed to be so or I do something wrong?

Future<void> cubitMain() async {
  print('----------CUBIT----------');

  /// Create a `CounterCubit` instance.
  final cubit = CounterCubit();

  /// Access the state of the `cubit` via `state`.
  print(cubit.state); // 0

  /// Interact with the `cubit` to trigger `state` changes.
  cubit.increment();

  await Future<void>.delayed(Duration.zero);

  final _sub = cubit.stream.listen((state) {
    print('Look, the state=$state');
  });

  await Future<void>.delayed(Duration.zero);

  cubit.increment();

  await Future<void>.delayed(Duration.zero);

  /// Access the new `state`.
  print(cubit.state);

  /// Close the `cubit` when it is no longer needed.
  await cubit.close();
  await _sub.cancel();
}

Log:

----------CUBIT----------
onCreate -- bloc: CounterCubit
0
onChange -- bloc: CounterCubit, change: Change { currentState: 0, nextState: 1 }
onChange -- bloc: CounterCubit, change: Change { currentState: 1, nextState: 2 }
Look, the state=2
2
onClose -- bloc: CounterCubit

felangel · 2022-08-19T14:26:02Z

Hi @sla-000 this is working as expected and has been this way since bloc v6.0.0. See https://bloclibrary.dev/#/migration?id=%e2%9d%97bloc-does-not-emit-last-state-on-subscription for more information.

sla-000 · 2022-08-20T15:47:11Z

Hi @felangel Thanks for quick response and explanation!
So from v6.0.0 if I want to subscribe a bloc to changes in other bloc I have to pass both current state value and a stream to this bloc and then
1 - process state, if there is one and it's must be checked
2- subscribe to stream and process new data in a listener

This creates boilerplate code. Is there any chance this ever will be changed back to BehaviorSubject or something similar? I understand you are trying to keep code pure with as little dependencies as possible but may be it is simplified too much and we are throwing the baby out with bath water 🙂

maximveksler · 2022-08-20T16:40:59Z

@sla-000 have you considered using a shared repository to implement communication between the blocks? This way it might simplify your use case of needing to cope with the duality of the bloc behaviour.

Note that BlocBuilder and BlocListener will perform correctly, for both 1st event and followup events.

sla-000 · 2022-08-20T16:44:01Z

@maximveksler Could you provide some example of that shared repository, please?

maximveksler · 2022-08-20T17:18:26Z

Yes, however I can't testify that it's the canonical example.

I will reference you the 2 examples, 1st is the firebase login where the authentication repository is shared between the App, Login and Signup BloC's. Pay attention that the App BloC and the Login Cubit listen to the user stream from the login repository.

flutter_firebase_login

User stream https://github.com/felangel/bloc/blob/master/examples/flutter_firebase_login/packages/authentication_repository/lib/src/authentication_repository.dart#L183
App BloC listen https://github.com/felangel/bloc/blob/master/examples/flutter_firebase_login/lib/app/bloc/app_bloc.dart#L22
Login BloC communication

bloc/examples/flutter_login/lib/login/bloc/login_bloc.dart

Line 55 in 997cfa8

await _authenticationRepository.logIn(

flutter_todos

The 2nd would be the Todo's example.

2 BloC's are listening for todo's https://github.com/felangel/bloc/blob/master/examples/flutter_todos/lib/stats/bloc/stats_bloc.dart#L24 & https://github.com/felangel/bloc/blob/master/examples/flutter_todos/lib/todos_overview/bloc/todos_overview_bloc.dart#L31
while another bloc is adding new ones https://github.com/felangel/bloc/blob/master/examples/flutter_todos/lib/edit_todo/bloc/edit_todo_bloc.dart#L52

Removed Rx dependency from bloc package

7d6df6b

mozhaiskyi requested a review from felangel as a code owner January 30, 2020 19:09

felangel assigned mozhaiskyi Jan 31, 2020

felangel added enhancement New feature or request pkg:bloc This issue is related to the bloc package labels Jan 31, 2020

felangel added this to In progress in bloc via automation Jan 31, 2020

felangel reviewed Jan 31, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

felangel reviewed Jan 31, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

felangel requested changes Jan 31, 2020

View reviewed changes

felangel requested a review from jorgecoca January 31, 2020 03:48

jorgecoca requested changes Jan 31, 2020

View reviewed changes

packages/bloc/pubspec.yaml Show resolved Hide resolved

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

mozhaiskyi added 2 commits January 31, 2020 16:49

Removed unnecessary broadcast stream

78de7e3

Merge branch 'master' into remove-rx-dependency

f92d137

mozhaiskyi force-pushed the remove-rx-dependency branch from 93632b2 to f92d137 Compare January 31, 2020 15:00

mozhaiskyi added 2 commits February 1, 2020 21:39

Implemented StateSubject

0af3a9d

Merge branch 'master' into remove-rx-dependency

8d31770

mozhaiskyi force-pushed the remove-rx-dependency branch from db319fa to 8d31770 Compare February 1, 2020 19:41

mozhaiskyi requested review from felangel and jorgecoca February 1, 2020 20:09

This was referenced Feb 1, 2020

Add multiple subscription tests to bloc #836

Closed

Decouple Bloc from RxDart #838

Closed

mozhaiskyi added 2 commits February 2, 2020 11:30

Merge branch 'master' into remove-rx-dependency

f5c4992

Simplified StateSubject solution

8086522

Small refactoring

af73012

felangel reviewed Feb 2, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

felangel reviewed Feb 2, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

felangel reviewed Feb 2, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

Renamed to _bindEventsToStates

67f8afe

felangel reviewed Feb 2, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Outdated Show resolved Hide resolved

Removed unnecessary .sink call

e22fe81

Co-Authored-By: Felix Angelov <felangelov@gmail.com>

felangel previously approved these changes Feb 2, 2020

View reviewed changes

tenhobi reviewed Feb 2, 2020

View reviewed changes

packages/bloc/lib/src/bloc.dart Show resolved Hide resolved

Fixed multi listeners problem

a2fca17

mozhaiskyi dismissed felangel’s stale review via a2fca17 February 2, 2020 19:32

mozhaiskyi requested a review from felangel February 2, 2020 19:35

brianegan reviewed Feb 2, 2020

View reviewed changes

felangel merged commit bc45e4d into felangel:master Feb 2, 2020

bloc automation moved this from In progress to Done Feb 2, 2020

felangel assigned brianegan, felangel and tenhobi Feb 2, 2020

felangel mentioned this pull request Feb 2, 2020

Remove RxDart dependency #839

Closed

3 tasks

netantho mentioned this pull request Feb 7, 2020

[Discussion] Maintaining Provider Dependency #712

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove RxDart dependency from bloc package #821

Remove RxDart dependency from bloc package #821

mozhaiskyi commented Jan 30, 2020 •

edited

Loading

felangel left a comment •

edited

Loading

tenhobi commented Jan 31, 2020

jorgecoca commented Jan 31, 2020

mozhaiskyi commented Jan 31, 2020

mozhaiskyi commented Feb 1, 2020

felangel commented Feb 1, 2020 •

edited

Loading

felangel commented Feb 2, 2020

mozhaiskyi commented Feb 2, 2020

felangel left a comment •

edited

Loading

brianegan commented Feb 2, 2020

brianegan commented Feb 2, 2020

brianegan Feb 2, 2020

felangel commented Feb 2, 2020

codecov bot commented Feb 2, 2020 •

edited

Loading

sla-000 commented Aug 19, 2022 •

edited

Loading

felangel commented Aug 19, 2022

sla-000 commented Aug 20, 2022 •

edited

Loading

maximveksler commented Aug 20, 2022

sla-000 commented Aug 20, 2022

maximveksler commented Aug 20, 2022 •

edited

Loading

Remove RxDart dependency from bloc package #821

Remove RxDart dependency from bloc package #821

Conversation

mozhaiskyi commented Jan 30, 2020 • edited Loading

Status

Breaking Changes

Description

felangel left a comment • edited Loading

Choose a reason for hiding this comment

tenhobi commented Jan 31, 2020

jorgecoca commented Jan 31, 2020

mozhaiskyi commented Jan 31, 2020

mozhaiskyi commented Feb 1, 2020

felangel commented Feb 1, 2020 • edited Loading

felangel commented Feb 2, 2020

mozhaiskyi commented Feb 2, 2020

felangel left a comment • edited Loading

Choose a reason for hiding this comment

brianegan commented Feb 2, 2020

brianegan commented Feb 2, 2020

brianegan Feb 2, 2020

Choose a reason for hiding this comment

felangel commented Feb 2, 2020

codecov bot commented Feb 2, 2020 • edited Loading

Codecov Report

sla-000 commented Aug 19, 2022 • edited Loading

felangel commented Aug 19, 2022

sla-000 commented Aug 20, 2022 • edited Loading

maximveksler commented Aug 20, 2022

sla-000 commented Aug 20, 2022

maximveksler commented Aug 20, 2022 • edited Loading

flutter_firebase_login

flutter_todos

mozhaiskyi commented Jan 30, 2020 •

edited

Loading

felangel left a comment •

edited

Loading

felangel commented Feb 1, 2020 •

edited

Loading

felangel left a comment •

edited

Loading

codecov bot commented Feb 2, 2020 •

edited

Loading

sla-000 commented Aug 19, 2022 •

edited

Loading

sla-000 commented Aug 20, 2022 •

edited

Loading

maximveksler commented Aug 20, 2022 •

edited

Loading