Coder Social home page Coder Social logo

feat: Add `force` to Emitter about bloc HOT 7 CLOSED

mcmah309 avatar mcmah309 commented on June 11, 2024 10
feat: Add `force` to Emitter

from bloc.

Comments (7)

felangel avatar felangel commented on June 11, 2024 2

Keeping avoiding re-emitting if newState == currentState as the base behavior makes sense and I am not arguing otherwise. This base behavior does encourage immutable state. But I'd disagree with

without precluding the use of mutable state.

To the extent that only allowing == to determine emitting does preclude mutable state as a sound design choice, when situation requires, since it makes a mutable state solution hacky.

Keeping in mind the base behavior is to only emit if newState == currentState, being explicit with emit(newState, force: true) is more apparent to maintainers on what effect this is having, rather than hiding that effect inside the == operator of the state.

Alas if I do override ==, as a developer, I shouldn't have to introduce a new variable like lastModified and change all my api's to interact correctly with this variable, just to work with bloc.

All that said, sounds like your position is firm, which is unfortunate since I know a good portion of the community wants/will benefit from having this option and it does not effect anyone who chooses not to use it. But at the end of the day, as the owner, it is up to you.

I would argue that adding force: true would be very confusing from an API stand-point because if we all agree that conceptually listeners should not be notified if there is no state change then why would anyone ever want to pass force?

If we can agree that blocs shouldn't notify listeners when a state has not changed, then I'm not sure why you feel overriding == is hiding an effect. Overriding == is just a way to customize whether or not two objects should be considered equal.

In the case that you described, if you want all states to be considered distinct then you should override == to always return false. If you don't want to override == then bloc will rely on the default referential equality comparison that Dart objects have to determine whether or not a state has changed.

Ultimately, I would argue bloc should never allow developers to disable the currentState == newState check because conceptually it does not make sense to notify listeners if nothing has changed.

You don't have to introduce a new variable like lastModified -- that was just an example. My point was you are in full control because bloc itself has no opinions about how you model your state, it only cares about notifying listeners when something has changed (and whether or not something has changed is fully in your control).

You can totally use bloc with mutable state if you really want to with no changes to the public API just as an example:

import 'package:bloc/bloc.dart';

mixin Distinct on Object {
  @override
  bool operator ==(Object value) => false;
}

class MyEvent {}

class MyState with Distinct {
  MyState({required this.value});

  int value;

  @override
  String toString() => 'MyState($value)';
}

class MyBloc extends Bloc<MyEvent, MyState> {
  MyBloc() : super(MyState(value: 0)) {
    on<MyEvent>((event, emit) {
      state.value++; // Look, we're mutating the state.
      emit(state); // And we're able to notify listeners.
    });
  }
}

void main() {
  MyBloc()
    ..stream.listen(print)
    ..add(MyEvent())
    ..add(MyEvent())
    ..add(MyEvent());
}

All that said, sounds like your position is firm, which is unfortunate since I know a good portion of the community wants/will benefit from having this option and it does not effect anyone who chooses not to use it.

I disagree with this for several reasons.

  1. I would argue the community has not expressed a strong interest in this as evidenced by the lack of requests for this functionality and this library's API being stable for many years now.
  2. As I mentioned above, the community already has the option to use mutable state if they choose (as evidenced by the example above).
  3. Introducing a new API does affect everyone because each time a developer calls emit they would now need to ask themselves additional questions (e.g. should I use force?, what does force mean?, why would I ever want to force?). Additionally, large teams would likely not want developers using force which would result in potentially having to create new lint rules to manage the usage of forceful emits, etc.

I hope this all helps provide more context regarding my reasoning. I take making changes to the public API of this library really seriously because there are many teams and developers out there who rely on the library and in order for a change to be justified it needs to be really clear that the change will benefit the majority of developers. In this case, I don't feel adding a force emit API would benefit the majority of developers -- in fact, I feel many existing users would not like this change for the reasons I mentioned above. I have also provided several examples of how the use-case you described can be achieved with no changes to the current API. Ultimately, whether a state change happens is 100% determined based on an equality comparison between the two states and as a developer you are in full control over this behavior.

from bloc.

tenhobi avatar tenhobi commented on June 11, 2024 1

I just feel like you should rather solve equity of your object rather than doing this? You can for example use Equatable for all object of your state representation and then it automatically should start to work, because your "same state", which is not actually the same state since you from what I understand mutated it inside, is now actually recognized as different automatically.

https://pub.dev/packages/equatable

from bloc.

mcmah309 avatar mcmah309 commented on June 11, 2024
  • It would be a very inefficient solution to traverse an entire file system to determine equality.
  • For an object, you may not want to track every field in == or hashcode. e.g. An id field of an object is used for fast retrieval/updating cache of correct object if fields change.
  • == or hashcode may not represent pure equality of all the fields. e.g. data structures that allow hashing different types in the same hash table or using keys of one type to retrieve another type.
  • Objects in a state may be outside your library to do not properly override ==.
  • The state object may contain objects significantly large. e.g. an entire document, copying would be inefficient.

There are a lot of reasons for allowing a force param. The framework shouldn't pigeon hole developers into a you must copy and compare, if you don't agree you have to leak emitter logic into the == operator of the state or switch to a different framework for this one scenario. Let the developer decide for the scenario. I am always in favor of giving developers more control. Especially when a solution like adding force to emit has zero overhead for the framework or effect on anyone who chooses not to use it.

from bloc.

tenhobi avatar tenhobi commented on June 11, 2024

I can see your point. I agree for same rare cases we might need the force parameter.

I would still argue however, than the documentation should advise people to handle equity properly in most cases, and only in rare cases such as complex and long objects or when using 3rd party objects that do not handle equity properly (or they also cannot) people should use only then force. Since proper equity might help with more things than only with emiting state.

// Of course a solution for now is to have some "fake" field inside of your object that you would set every time when you try to emit. (if I understand correctly otherwise you have the same equity)

from bloc.

felangel avatar felangel commented on June 11, 2024

Hi @mcmah309 👋
Thanks for opening an issue!

The only thing bloc does is avoid re-emitting if newState == currentState since it conceptually does not make sense to notify listeners if nothing has changed. I'd prefer to keep this behavior since it's been a core concept within bloc since the library was created and encourages using immutable state as the default without precluding the use of mutable state.

I currently have ephemeral virtual file system structures inside my bloc's state. Each time the file system changes, it would be ridiculous to copy the entire virtual file system structure.

If you want to keep a filesystem as part of a bloc state (which I would not necessarily recommend), then I would argue it could make sense to override == of your state (not your FileSystem object) to meet your requirements.

One could override the == operator on the state object to always return false, but this is not a desired solution because this would be abuse of ==, since == would return false, indicating that the objects are different, but it does not actually mean they are different. This would be leaking emitter logic into the state and making use of == anywhere else in different contexts (like a hashmap of file system nodes) yield wrong results.

I would not consider this abuse because as I mentioned above, the custom == operator would apply to the state of your bloc (not the FileSystemobject itself).

For example:

@immutable
class MyState {
  const MyState(this.fileSystem);

  final FileSystem fileSystem;

  @override
  bool operator ==(Object other) {
    return other is MyState &&
        other.runtimeType == runtimeType &&
        other.fileSystem.lastModified == fileSystem.lastModified;
  }

  @override
  int get hashCode => Object.hashAll([fileSystem]);
}

In the above example, mutating the file system would result in its lastModified timestamp changing and could be used in the custom equality comparison. I would not consider this abuse because the FileSystem equality is unaffected and the state equality comparison reflects your requirements (two states should be considered equal if their lastModified timestamps match as opposed to the built-in referential equality comparisons for Dart objects). This is just an example to illustrate my point but you can obviously use whatever properties you want to determine whether two states should be considered equal.

I would prefer not to introduce a new force API because it's already possible to support the use-case you have with the current APIs. In addition, if you are maintaining such a large object as part of your bloc state, then it might be a sign your bloc is doing too much and/or you might want to consider moving that information out of the bloc state.

Hope that helps, thanks! 👍

from bloc.

mcmah309 avatar mcmah309 commented on June 11, 2024

Keeping avoiding re-emitting if newState == currentState as the base behavior makes sense and I am not arguing otherwise. This base behavior does encourage immutable state. But I'd disagree with

without precluding the use of mutable state.

To the extent that only allowing == to determine emitting does preclude mutable state as a sound design choice, when situation requires, since it makes a mutable state solution hacky.

Keeping in mind the base behavior is to only emit if newState == currentState, being explicit with emit(newState, force: true) is more apparent to maintainers on what effect this is having, rather than hiding that effect inside the == operator of the state.

Alas if I do override ==, as a developer, I shouldn't have to introduce a new variable like lastModified and change all my api's to interact correctly with this variable, just to work with bloc.

All that said, sounds like your position is firm, which is unfortunate since I know a good portion of the community wants/will benefit from having this option and it does not effect anyone who chooses not to use it. But at the end of the day, as the owner, it is up to you.

from bloc.

felangel avatar felangel commented on June 11, 2024

Closing this for now but happy to continue the discussion if anyone wants to weigh in or feels strongly about this, thanks!

from bloc.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.