Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignore emit in closed cubit #4187

Closed
wants to merge 2 commits into from
Closed

ignore emit in closed cubit #4187

wants to merge 2 commits into from

Conversation

ra48ad
Copy link

@ra48ad ra48ad commented Jun 10, 2024

Status

IN DEVELOPMENT

Breaking Changes

YES

Description

This change will solve the inconsistency between Bloc and Cubit if emit is used after the Bloc/Cubit is closed. (Issue #4165)
The added emit override makes sure no error is thrown if emit is called and the cubit is already closed. Instead, the emit will be simply ignored just like a Bloc.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@ra48ad ra48ad requested a review from felangel as a code owner June 10, 2024 21:48
Copy link
Owner

@felangel felangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right change. The bloc and cubit behavior is inconsistent because there is a slight delay when adding an event due to the Event stream/sink not because the emit behavior is different. If we are to adjust this it should likely be in BlocBase. I'm going to close this for now since I want to take some time to consider the implications of this change before proceeding.

@felangel felangel closed this Jun 11, 2024
@ra48ad ra48ad deleted the ignore_emit_in_closed_cubit branch June 12, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants