Skip to content

Conversation

@pr0gramista
Copy link
Contributor

Status

READY

Breaking Changes

NO

Description

Fixes #2127
Closes #2181

To make a builder, listener or consumer update when the instance is changed we need a context dependency. However we can't use normal context.watch because BlocProvider uses a custom provider which also updates dependents when state is changed, therefore it would make buildWhen useless. Ideally we would reverse that, but that would also break compatibility. Adding another "normal" provider is also not an option since provider does not allow it and integrating special InheritedWidget is tricky.

There is a concept of "aspect" when it comes to dependencies. Provider exposes it by context.select, but it is a bit different and it can't be used in didUpdateDependencies, but we can put it in build and use it to target only the changes of the instance by retrieving hashCode.

I think for people who doesn't change instances in the runtime this makes almost no difference apart from dependency.

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

@pr0gramista pr0gramista requested a review from felangel as a code owner May 27, 2021 12:49
@pr0gramista pr0gramista changed the title Add dependency to bloc instance for BlocBuilder/BlocListener/BlocConsumer fix(flutter_bloc): Add dependency to bloc instance for BlocBuilder/BlocListener/BlocConsumer May 27, 2021
@felangel felangel added bug Something isn't working pkg:flutter_bloc This issue is related to the flutter_bloc package labels May 27, 2021
@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

Merging #2482 (01ecf30) into master (6d8a0b8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2482   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          118       139   +21     
=========================================
+ Hits           118       139   +21     
Impacted Files Coverage Δ
packages/flutter_bloc/lib/src/bloc_builder.dart 100.00% <100.00%> (ø)
packages/flutter_bloc/lib/src/bloc_consumer.dart 100.00% <100.00%> (ø)
packages/flutter_bloc/lib/src/bloc_listener.dart 100.00% <100.00%> (ø)

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 1c5b2cb...01ecf30. Read the comment docs.

@pr0gramista
Copy link
Contributor Author

@felangel seems like very_good_analysis got an update yesterday and this one example doesn't pass CI anymore


@override
Widget build(BuildContext context) {
if (widget.bloc == null) context.select<B, int>(identityHashCode);
Copy link
Owner

Choose a reason for hiding this comment

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

It's a very creative idea to solve the previous issue! I made one minor adjustment to use identityHashCode in case developers override hashCode manually.

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.

Thanks so much for taking the time to address this issue! I really appreciate the creative solution and will try to get this merged shortly 💙 🙏

@felangel
Copy link
Owner

@felangel seems like very_good_analysis got an update yesterday and this one example doesn't pass CI anymore

Yup #2483 should resolve this 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pkg:flutter_bloc This issue is related to the flutter_bloc package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BlocBuilder continues to use old cubit instance when the context is updated with a new instance

2 participants