Bloc: [BREAKING][Proposal] remove initialState override

[felangel]

Is your feature request related to a problem? Please describe.
As a developer, having to override initialState when creating a bloc presents two main issues:

1. The initialState of the bloc can be dynamic and can also be referenced at a later point in time (even outside of the bloc itself). In some ways, this can be viewed as leaking internal bloc information to the UI layer.
2. It's verbose.

Describe the solution you'd like
Rather than defining the initial state like:

class CounterBloc extends Bloc<CounterEvent, int> {
  @override
  int get initialState => 0;

  ...
}

I am proposing:

class CounterBloc extends Bloc<CounterEvent, int> {
  CounterBloc() : super(0);

  ...
}

This will avoid exposing initialState publicly and also reduces the amount of code needed especially considering in most cases a bloc will already require a constructor for injecting a repository like:

class MyBloc extends Bloc<MyEvent, MyState> {
  MyBloc(this._myRepository);

  final MyRepository _myRepository;

  @override
  MyState get initialState => MyState.initial();
}

The above case could be simplified to:

class MyBloc extends Bloc<MyEvent, MyState> {
  MyBloc(this._myRepository) : super(MyState.initial());

  final MyRepository _myRepository;
}

Additionally, it would no longer be possible to dynamically modify the value of a bloc's initialState after it has been initialized.

In the current state, the following code is totally valid but could potentially have many unintended side-effects

class CounterBloc extends Bloc<CounterEvent, int> {
  @override
  int get initialState => _initialState;

  int _initialState = 0;

  @override
  Stream<int> mapEventToState(CounterEvent event) async* {
    switch (event) {
      case CounterEvent.increment:
        _initialState = 100; // this should ideally not be possible
        yield state + 1;
        break;
      default:
        break;
    }
  }
}

Additional context
This would also enable reduction in boilerplate for HydratedBloc

Before:

class CounterBloc extends HydratedBloc<CounterEvent, int> {
  @override
  int get initialState => super.initialState ?? 0;

  ...
}

We can simplify the API and eliminate the need to explicitly call super.initialState

class CounterBloc extends HydratedBloc<CounterEvent, int> {
  CounterBloc() : super(0);

  ...
}

Would love to hear everyone's feedback on this especially if you are specifically relying on initialState for something outside of just seeding the bloc's initial state 👍

Looking forward to hearing everyone's thoughts 😄

UPDATE
After going through a lot of feedback it seems a good alternative would be to use a named parameter in super.

class CounterBloc extends Bloc<CounterEvent, int> {
  CounterBloc() : super(initialState: 0);

  ...
}

[ThinkDigitalSoftware]

LGTM. I don't see the modification as an issue. But less boilerplate is fine with me!

[ram231]

Would like to see the major changes. It should be notified to all the users that the initialState is @deprecated so that they can easily migrate to the newer version. But having to refactor individual blocs is cumbersome and should have a temporary build runner to migrate from initialState to super(MyInitialState())

[narcodico]

Hi @felangel 👋

Thanks for opening an issue! 😂
I like it, less boiler plate, cleaner API ✔🥳

[kranfix]

The proposed syntax is, probably, the most intuitive. I saw many newbies' question about how to initiate the state thinking in this proposal way.

Respective to the breaking change, refactoring won't be a big extra effort. Then I agree with the proposal.

[andredsnogueira]

Looks good. Less boilerplate is 🔥

[nhwilly]

You the man. I completely trust your judgement.

But does this mean I could pass in an instance of, say, a user object into the initial state?

That could be really handy.

[CodingAleCR]

Eventhough it be a huge change for my apps I believe that this change makes the code easier to understand and also as @nhwilly said it could be very handy to be able to pass instances to a bloc. Nice thinking!

[binoypatel]

Like the approach but breaking changes will result in the overhead, but I think it will be a good tradeoff

[chimon2000]

Related: https://github.com/felangel/cubit/issues/6

I think there's some value in that this allows teams to scale capabilities without drastically changing underlying dependencies. If you're already using cubit, and want the abilities of bloc for a feature, the change is minimal.

[ZeBrody]

Sounds good and at first glance i don't see any backdraws.

I sometime use a constructor like this:

class MyBloc extends Bloc<MyEvent,MyState> {
  MyBloc({this._myModel});
  MyModel _myModel;

  @override
  MyState get initialState() {
    if(_myMode==null) {
      return MyStateLoading();
    } else {
      return MyStateLoaded(_myModel);  
   }
  }
}

But that could easily be changed to a ternary operator in the super call.

[tenhobi]

The third possible way is using the new late keyword, i.e. (note it's in Bloc class, not MyBloc class)

class Bloc ... {
  late final initialState;
}

But this brings more responsibility to user to actually set the initial state before using it.

I would personally go for the way with a constructor. Maybe even use it as named parameter? MyBloc(): super(initialState: MyInitialState()) That might help the API when we add something in the future.

[albertodev01]

I think that removing initialState and replacing it with a constructor is fine but maybe it should be a named one. Since nndb is going to have the required keyword it can be both named and needed.

---

However, personally, I don't see a problem with the current state of things because:

1. In terms of amount of code to write, Android Studio generates the methods to be ovverridden for me. No problems here and even if it didn't, it would just be a single line.

2. The state can still be changed even with constructors because people might not instantiate the object in place.

My reasoning in point 2 is the following. Instead of writing this...

MyBloc(
  initialState: InitialState(),
);

... one could make this ...

final state = InitialState(),
MyBloc(
  initialState: state,
);

... and the state variable would point to the "internals" of the BLoC. It's no different from having a public getter which exposes a value to the outside!

Someone using the BLoC must know that the initial state doesn't have to be modified after the instantiation of the bloc. With this knowledge, both the constructor way and the getter way are fine in my opinion because the user has the responsibility to not touch the internals.

If any state had a copyWith method there wouldn't be this issue because even if initialState were changed later, you "stored" a copy of it (somewhere). So at the end, I'd say that removing initialState in place of a constructor is a nice idea but personally I don't think it's strictly required!

There will also be the need for a refactor of course and I don't think it would take too long, but still it has to be taken into account

[StefanYYC]

LGTM, I don't remember having to change the value of the initialState.
After maybe it can be useful to let it optionnal in case you need to do specific things on it...
But to me it's simply less code for the same result so it's a big yes!

[neil-dev]

Well i feel that initialstate getter clearly defines the purpose, the new super declaration appears more compact. Well, less code is better, right?

[fotiDim]

Can we have a named argument in super()?

[zepfietje]

As @tenhobi and @fotiDim mentioned, it might be a good idea to consider making initialState a named parameter in the constructor. Aside from compatibility with possible future changes, I believe it makes the code quite a bit clearer.

[DeadlyMissile]

In Felix we trust 👍❤️

[VKBobyr]

I don't know if I'm misusing the bloc pattern, but I use the initialState when initializing a custom Navigator like so:

return BlocProvider<CoreBloc>.value(
  value: coreBloc,
  child: Navigator(
    key: coreBloc.router.navigator,
    // initial state used here 
    initialRoute: coreBloc.router.stageToName[coreBloc.initialState.stage],
    onGenerateRoute: coreBloc.router.onGenerateRoute,
  ),
);

Though I guess I could instantiate the router with the initialState from the beggining

[felangel]

@VKBobyr you could always expose your own public initialState or as you mentioned instantiate the router with the initialState from the very beginning to achieve the same thing but can you please elaborate a bit on the responsibility of the CoreBloc? I wouldn't recommend having a router as a property of a bloc since routing is more of a presentation thing that is tied to the platform.

[VKBobyr]

@felangel Yeah, decoupling router and bloc would certainly solve the issue and is probably a better design choice anyway.

I do support the change then!

[andesappal]

LGTM. I don't see the modification as an issue. But less boilerplate is fine with me!

Definitely is an issue if it causes a leak. Thanks for the fix, Felix!

[felangel]

It looks like having a required, named parameter isn't ideal either because there won't be any warning/error if you forget to add it until NNBD (related issue: https://github.com/dart-lang/sdk/issues/42438).

For example,

class CounterBloc extends Bloc<CounterEvent, int> {
  // Code will compile without the following line
  CounterBloc() : super(initialState: 0);

  @override
  Stream<int> mapEventToState(CounterEvent event) async* {}
}

The above snippet will compile with no warnings/errors and will result in the bloc having a null initial state which in many cases will be undesirable.

This will be resolved once NNBD lands in stable but until then I think it will be safer to use a non-named parameter like:

class CounterBloc extends Bloc<CounterEvent, int> {
  // Code will not compile without the following line
  CounterBloc() : super(0);

  @override
  Stream<int> mapEventToState(CounterEvent event) async* {}
}

This will ensure that an initial state is always specified (even if it's null).

[kranfix]

@felangel what about using @required and assert(initialState != null) in case that syntax was the most liked?

[felangel]

@kranfix I think we want to support nullable states (https://github.com/felangel/bloc/issues/1312) so asserting is not desirable plus it still would result in an exception at run-time rather than compile-time which I would like to avoid.

[ram231]

I think the @required would be good enough. So that Developers know what initial state they put in the Bloc.

[ZeBrody]

Having @required isn't a solution here, since it won't even show a warning if you don't call super in a Constructor.

On the other hand, if null is a valid initial state, should it actually be required?

Given the great documentation for this package, it isn't hard for a developer to figure out how to set the initial state.
And when the developer has a specific initial state intended, it's unlikely that he will just forget to set it.

[ram231]

Having @required isn't a solution here, since it won't even show a warning if you don't call super in a Constructor.

Would be great if there are linters to have similar way as @mustCallSuper decorator.

On the other hand, if null is a valid initial state, should it actually be required?

If the developer explicitly put null there so that he knows what he/she put in the constructor.

Given the great documentation for this package, it isn't hard for a developer to figure out how to set the initial state.

The documentation is great. If people enforce to have naming conventions like TodoInitialState or TodosLoadingState, then probably there's no reason to have named parameters

And when the developer has a specific initial state intended, it's unlikely that he will just forget to set it.

In my earlier comment, I did not specify named parameters to be the solution. when i said I think, I am just suggesting ideas. But thanks for justifying it.

[TomEversdijk]

I am struggling with this update.

How can I call super when there is logic involved in setting the initialState?

e.g.

class MyBloc extends Bloc< MyEvent, MyState > {
  OtherBloc otherBloc;
  MyBloc({@required this.otherBloc});

  @override
  MyState get initialState {
    final state = otherBloc.state;
    if (state is OtherStateFoo) {
      return MyStateFoo;
    } else {
      return MyStateBar;
    }
  }
}

Ugly solution 1:
Ofcourse I can do this initialState logic before initialising the Bloc and passing the result as a parameter to the bloc, but that cannot be an improvement of the API right?

Ugly solution 2:
An other option would be to make a static initialState method as static functions are callable before calling the super method. In my opinion this wouldn't improve the API either.

class MyBloc extends Bloc< MyEvent, MyState > {
  OtherBloc otherBloc;
  MyBloc({@required this.otherBloc}): super(initialState(otherBloc));

  static MyState initialState (OtherBloc staticOtherBloc){
    final state = staticOtherBloc.state;
    if (state is OtherStateFoo) {
      return MyStateFoo;
    } else {
      return MyStateBar;
    }
  }
}

Any other suggestions?

[felangel]

@TomEversdijk you can do:

import 'package:bloc/bloc.dart';
import 'package:meta/meta.dart';

abstract class MyOtherEvent {}

abstract class MyOtherState {}

class MyOtherStateA extends MyOtherState {}

class MyOtherStateB extends MyOtherState {}

class MyOtherBloc extends Bloc<MyOtherEvent, MyOtherState> {
  MyOtherBloc() : super(MyOtherStateA());

  @override
  Stream<MyOtherState> mapEventToState(MyOtherEvent event) async* {}
}

abstract class MyEvent {}

abstract class MyState {}

class MyStateA extends MyState {}

class MyStateB extends MyState {}

class MyBloc extends Bloc<MyEvent, MyState> {
  final MyOtherBloc myOtherBloc;
  MyBloc({@required this.myOtherBloc})
      : super(myOtherBloc.state is MyOtherStateA ? MyStateA() : MyStateB());

  @override
  Stream<MyState> mapEventToState(MyEvent event) async* {}
}

Hope that helps 👍