Is your feature request related to a problem? Please describe.
We are using the .bloc() and .repository() extension in our Widget.build methods. Now that these methods are deprecated in favour of .watch() and .read() our current workflow is no longer possible.
We don't really want to watch (or select) them because we know they won't change (repositories) or we just need a function reference (blocs). And read is not allowed in a build method.
The documentation of bloc also still has the following:
then from either
ChildA, orScreenAwe can retrieveBlocAwith:context.read<BlocA>();
This is not fully the case anymore as it's not possible from within the build method.
I think it's confusing that context.read<BlocA>() is not allowed but BlocProvider.of<BlocA>(context) is working perfectly.
(A similar inconsistency exists in provider because Provider.of(context, listen: false) is allowed in a build while .read is not)
Describe the solution you'd like
Considering that .bloc() and .repository() have a different behaviour than .read() and watch(), it might be worth it to keep them? (Although I can understand the confusion if so)
Describe alternatives you've considered
context.bloc() and context.repository() calls to BlocProvider.of() RepositoryProvider.of().bloc() and .repository() extension functions in our own project.Hi @petermnt 馃憢
Thanks for opening an issue!
Can you share some example usage? You should not be using context.read to access a bloc directly in build ideally because context.read should only be used when you need to access a bloc to add an event or access a repository to inject into another bloc:
If the bloc is used for adding events you should access it directly within the closure where the event is added like:
TextButton(
onPressed: () {
context.read<MyBloc>().add(MyEvent();
},
);
If the repository is accessed to inject into a bloc you should access it within the create method like:
BlocProvider(
create: (context) => MyBloc(context.read<MyRepository>()),
child: ...
);
You should avoid using context.bloc or context.repository directly in build because it is inefficient (the lookup happens on each rebuild and in many cases is not required.
If you are using context.bloc<MyBloc>().state in your build method, then that should be replaced with context.watch in order to ensure that the widget rebuilds when the bloc state changes (similar to BlocBuilder).
I think it's confusing that context.read
() is not allowed but BlocProvider.of (context) is working perfectly.
(A similar inconsistency exists in provider because Provider.of(context, listen: false) is allowed in a build while .read is not)
This is by design because flutter_bloc is using provider but in general you shouldn't need to do that.
Considering that .bloc() and .repository() have a different behavior than .read() and watch(), it might be worth it to keep them? (Although I can understand the confusion if so)
The behavior is different but leads to bad habits and the goal was to promote safer, more efficient code while also aligning with existing standards from package:provider.
I'm happy to take a look at any and all scenarios which you feel are no longer compatible and I'm working on adding details notes to the migration guide.
Let me know if that helps and sorry for any inconvenience or confusion! 馃檹
Honestly, the reason we use context.bloc in the first place because it is more convenient and much simpler than writing BlocProvider.of(). At least that was my case in my team.
I think something that we should change is the deprecated message. It shouldn't say:
'Use context.read or context.watch instead. Will be removed in v7.0.0'
Because when I refactored all context.bloc to context.read, I automatically assume it's ok to use it inside Widget.build method like we used to. Why don't we give a small hint on the error message to use BlocProvider.of()? Considering context.bloc and context.read it's not exactly the same under the hood.
Thanks for your reply.
We have 2 main uses for it.
For the .bloc it's usually something like this:
@override
Widget build(BuildContext context) {
final bloc = context.bloc<ListBloc>();
return BlocBuilder<ListBloc, ListState>(
cubit: bloc,
builder: (context, state) => state.when(
loading: (_) => // cut,
data: (viewModels, _, __) => // cut,
fullScreenError: (message, _) => ErrorPlaceholder(text: message, onTap: bloc.refresh),
empty: (_) => // cut,
),
);
}
It's a very slight performance optimisation to not retrieve the bloc twice. I do realise that this can be circumvented by using onTap: () => context.read<ListBloc>().refresh instead of onTap: bloc.refresh though. We were just trying to avoid an unnecessary lambda.
Sometimes we also pass the Bloc instance to a (private) child widget, for similar reasons.
So this problem we can work around with a negligible performance difference.
For the Repository's it's a bit more complicated. We actually use RepositoryProvider quite heavily as a DI tool, also in widgets, so we can change some of these implementations for our widget and test-driver testing.
In a few places this is to access some real repositories directly in widgets where using a bloc is overkill, but mostly it's to have easy access to several object which contain shared functions/information that does not change at runtime (custom theme objects, a class that handles all our navigation, Platform info, version info, etc).
Do you have any suggestions for improvements here?
Honestly, the reason we use
context.blocin the first place because it is more convenient and much simpler than writingBlocProvider.of(). At least that was my case in my team.I think something that we should change is the deprecated message. It shouldn't say:
'Use context.read or context.watch instead. Will be removed in v7.0.0'
Because when I refactored all
context.bloctocontext.read, I automatically assume it's ok to use it insideWidget.buildmethod like we used to. Why don't we give a small hint on the error message to useBlocProvider.of()? Consideringcontext.blocandcontext.readit's not exactly the same under the hood.
Because you shouldn't need to use BlocProvider.of(). If you're seeing errors it's because you are either using context.read within build directly or context.watch outside of build.
@petermnt no problem!
Yeah for the first case I'd recommend:
@override
Widget build(BuildContext context) {
return BlocBuilder<ListBloc, ListState>(
builder: (context, state) => state.when(
loading: (_) => // cut,
data: (viewModels, _, __) => // cut,
fullScreenError: (message, _) => ErrorPlaceholder(text: message, onTap: () => context.read<ListBloc>().refresh()),
empty: (_) => // cut,
),
);
}
For the Repository's it's a bit more complicated. We actually use RepositoryProvider quite heavily as a DI tool, also in widgets, so we can change some of these implementations for our widget and test-driver testing.
Can you please provide some example usage that you are unsure how to refactor? I'm not sure I fully understand the issue based on this description, thanks! 馃憤
Can you please provide some example usage that you are unsure how to refactor? I'm not sure I fully understand the issue based on this description, thanks! 馃憤
It's pretty straightforward.
On the app level we have 1 MultiProvider, above our root navigator, which provides many app level objects.
One example is RepositoryProvider(create: (_) => ThemeBuilder()),
And then in almost all of our widgets we do final theme = context.repository<ThemeBuilder>(); to retrieve our custom theme objects with colors, styles, etc.
The most straightforward way to refactor this is just having a singleton or an actual InheritedWidget, just going to take some work and is less convenient for testing, and we'd lose the convenience of MultiProvider, but it's definitely possible.
Any other tips are very welcome though.
@petermnt in that case you should be able to use context.watch like:
final theme = context.watch<ThemeBuilder>();
Hope that helps 馃憤
@petermnt in that case you should be able to use
context.watchlike:final theme = context.watch<ThemeBuilder>();Hope that helps 馃憤
Yeah that should work, do you maybe know if there's any performance impact for the implicit listen: true instead of listen: false?
@petermnt there should not be any significant performance impact especially when the ThemeBuilder instance does not change 馃憤
Ok thanks, then we can easily change it like that.
While refactoring I ran into 1 very specific case which I can't solve without the use of BlocProvider.of inside the build.
If this is the only thing that's not possible I don't mind just calling it manually like that, but I'd just like to let you know to give some insight into weird use-cases.
We have a ListBloc mixin which gets used in a generic ListScreen with a BlocBuilder<ListBloc, S>.
For 1 screen we have a PodcastDetailBloc which uses the mixin ListBloc.
In the main widget we want to have a BlocBuilder<PodcastDetailBloc, S> while the listscreen has BlocBuilder<ListBloc, S>.
We were able to manage it like this:
final bloc = context.bloc<ListBloc>() as PodcastDetailBloc;
return BlocBuilder<PodcastDetailBloc, ListState<ExtraData>>(
cubit: bloc,
buildWhen: (prev, curr) => prev.extraData?.shareUrl != curr.extraData?.shareUrl,
builder: (context, state) => Scaffold(
appBar: AppBar(
actions: [
if (state.extraData?.shareUrl != null) ShareIcon(),
],
),
),
body: CompositionListScreen(),
),
);
Would you suggest to use a watch here instead too?
@petermnt can you share how you're providing the PodcastDetailBloc? I think it should be possible to remove the manual lookup
return BlocBuilder<PodcastDetailBloc, ListState<ExtraData>>(
buildWhen: (prev, curr) => prev.extraData?.shareUrl != curr.extraData?.shareUrl,
builder: (context, state) => Scaffold(
appBar: AppBar(
actions: [
if (state.extraData?.shareUrl != null) ShareIcon(),
],
),
),
body: CompositionListScreen(),
),
);
If that's not possible you can use BlocProvider.of<ListBloc>(context) as PodcastDetailBloc; but I think it should not be necessary.
@petermnt can you share how you're providing the
PodcastDetailBloc? I think it should be possible to remove the manual lookupIf that's not possible you can use
BlocProvider.of<ListBloc>(context) as BlocProvider<ListBloc>;but I think it should not be necessary.
We are providing it very specifically as BlocProvider<ListBloc> because the child widget tries to retrieve it through BlocBuilder<ListBloc, S>. If we provide it as BlocProvider<PodcastDetailBloc> the child can't find it for its BlocBuilder.
But it's not a big problem, I understand this is a very exceptional case.
@petermnt why does it need to be provided as BlocProvider<ListBloc> if it's being accessed as PodcastDetailBloc?
@petermnt why does it need to be provided as
BlocProvider<ListBloc>if it's being accessed asPodcastDetailBloc?
It's being accessed by CompositionListScreen as ListBloc and by the pasted widget as PodcastDetailBloc
@petermnt why can't the CompositionListScreen access it as PodcastDetailBloc?
@petermnt why can't the
CompositionListScreenaccess it asPodcastDetailBloc?
That screen is used by many other screens, it has no knowledge of PodcastDetailBloc.
Yeah I guess I don't have enough context so I'll leave it up to you 馃槃
I would recommend using BlocProvider.of in this case since you don't care about the state of the bloc. context.watch should only be used when you want to access/react to changes in the bloc state (similar to BlocBuilder).
Yeah I guess I don't have enough context so I'll leave it up to you 馃槃
You can either usecontext.watchorBlocProvider.of. I would recommend usingBlocProvider.ofin this case since you don't care about the state of the bloc.
No problem. Small effort to keep the BlocProvider.of there.
Thanks for your time. While it's a little bit less convenient for us, I do understand the reason for the changes.
@petermnt I'm really sorry for the inconvenience! This is just a deprecation so you can feel free to migrate your codebase at your own pace -- v7.0.0 is still a long way off haha.
Let me know if there's anything else I can do to help. I've published some additional documentation regarding these changes in the migration guide.
@petermnt I'm really sorry for the inconvenience! This is just a deprecation so you can feel free to migrate your codebase at your own pace -- v7.0.0 is still a long way off haha.
Our CI enforced analysis check does not let me do that. But we did that to ourselves 馃槃
Let me know if there's anything else I can do to help. I've published some additional documentation regarding these changes in the migration guide.
I've read the migration guide and the intention and standard migration are pretty clear.
We will reconsider our overly extensive use of RepositoryProvider directly in widgets and for the rest the migration is pretty straightforward.
Thanks again.