Provider: context.read crash when call in build function.

[tbm98]

provider version 4.1.0-dev+2
'package:provider/src/provider.dart': Failed assertion: line 428 pos 7: 'debugIsInInheritedProviderCreate || (!debugDoingBuild && !debugIsInInheritedProviderUpdate)': is not true.
my code

import 'package:flutter/material.dart';
import 'package:provider/provider.dart';

void main() => runApp(MyApp());

class M with ChangeNotifier {
  int a = 0;
  int b = 0;

  void incre() {
    a++;
    notifyListeners();
  }
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
          appBar: AppBar(
            title: Text('hi'),
          ),
          body: MultiProvider(providers: [
            ChangeNotifierProvider(
              create: (_) => M(),
            )
          ], child: MyCustomForm())),
    );
  }
}

class MyCustomForm extends StatefulWidget {
  @override
  MyCustomFormState createState() {
    return MyCustomFormState();
  }
}

class MyCustomFormState extends State<MyCustomForm> {
  @override
  Widget build(BuildContext context) {
    print('rebuild main');
    return Scaffold(
      body: Center(
        child: Column(
          children: <Widget>[
            btn(),
            tx(),
          ],
        ),
      ),
    );
  }
}

class btn extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    int a = context.read<M>().b;
    print('rebuild btn');
    return RaisedButton(
      onPressed: () {
        context.read<M>().incre();
      },
      child: Text('incre'),
    );
  }
}

class tx extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    print('rebuild text');
    return Text('hi ${context.watch<M>().a}');
  }
}

[mono0926]

int a = context.read<M>().b;

int a = context.watch<M>().b; is correct.

read() can be used only if !debugDoingBuild && !debugIsInInheritedProviderUpdate is satisfied.
But debugDoingBuild is true during build phase.

And this is better:

int a = context.select((M m) => m.b);

[tbm98]

But why it work ok if use int a = Provider.of<M>(context,listen: false).b;
I think context.read and Provider.of(context,listen: false) are equivalent to each other 🤔

[rrousselGit]

Extensions adds extra assertions to make things safer.

The assertions are not added on Provider.of, in case you trust what you're doing.

[tbm98]

That's just a test of mine and probably won't show up in real life.

[esDotDev]

Can these be removed? I know exactly what I'm doing, but having this blow up in one case and not the other makes no sense.

var s = Provider.of<MainScaffoldState>(context, listen: false); //Works
var s2 = context.read<MainScaffoldState>(); //Yells at me, even though it literally just wraps the above fxn

I just wasted close to an hour thinking I was doing something wrong, as it never even occurred to me you would be adding asserts that Provider.of<> does have.

In this case, my state has several ValueNotifiers in it, I would like to just pass the state down, and bind various sub-trees to these ValueNotifiers.

 /// Grab reference to state so we can bind to one of it's ValueNotifiers, and call handler functions 
 MainScaffoldState state = context.read();
/// Bind to a property on state
 return ValueListenableBuilder(
     valueListenable: state.currentPageNotifier,
     builder(_, PageType page, __) => 
          MyWidget(pageType: page, onPressed: ()=> state.doStuff())

I don't see why this approach would be discouraged. Yes I could create a dedicated ChangeNotifier to hold all these props, and use Selector, but that does nothing except add boilerplate and obfuscation. With the above approach, I do not have to create a dedicated model, and I do not need to create a bunch of wrapper functions that call notifyListeners, saving dozens of lines of code. I previously had it architected that way, and just finished re-factoring cause I didn't like the amount of boilerplate it was producing.

If Provider is truly just a tool as you said in your latest talk, I think it would make sense to keep these opinions out of it as much as possible. As you said, it's InheritedWidgets made easy, that's kinda where the opinions should end imo, please keep it flexible.

[rrousselGit]

I know exactly what I'm doing, but having this blow up in one case and not the other makes no sense.

The documentation is clear about the fact that this is done on purpose.
context.read/watch/select are stricter. If you don't want that, use the good old Provider.of

The reason being, calling .read inside build is unsafe and should not be done. Instead use either .watch or .select.

[esDotDev]

So not open to re-considering at all? I feel like I've presented a pretty valid, and not at all 'unsafe' use case above. If I'm going to switch to using the context methods, I would prefer to switch fully, and not to have to randomly have Provider.of() littered around the code base.

This is a very arbitrary decision. If it is so 'unsafe' then why not add the asserts to main .of()? If it's not really that unsafe, then why not allow it with the extension?

I fundamentally don't get this outlook of treating developers in this paternalistic way. We're not children. I can obviously just make our own extensions, but this is a _very_ weird position to take imo.

[esDotDev]

Also curious why it is at all unsafe... you're enforcing the _listen:false_ isn't that the definition of the safe way to access the provided value, without getting bound for rebuilds?

What is unsafe here? Just that they will accidentally use _read_ instead of _watch_ and not get rebuilds?

[rrousselGit]

Just that they will accidentally use read instead of watch and not get rebuilds?

Basically, yes.
Using read inside build is very brittle. If you're looking to optimize rebuilds, provider has select for that instead.

If it is so 'unsafe' then why not add the asserts to main .of()?

So that people can make their own extensions with their own rules if they want to. And to not make a breaking change

[esDotDev]

So to account for people who have close-to-zero understanding of how the api work, we're limiting use cases for those who do? Read vs Watch is not an ambiguous distinction. It's actually much more explicit now, than it was before with an optional listen: param that many people did not realize exists.

What I don't like about this, is if you want to share some top-level state, with several sub-widgets, you're forced to create a Model that Extends ChangeNotifier and pass that down. But it's a nice option, to just be able to pass down the state itself, and avoid that boilerplate of creating the extra class.

It's just the difference between this:

class MyWidget extends StatefulWidget {
  @override
  MyWidgetState createState() => MyWidgetState();
}

class MyWidgetState extends State<MyWidget> {
  ValueNotifier<int> tabIndexNotifier = ValueNotifier(0);

  @override
  Widget build(BuildContext context) {
    return Provider.value(value: this, child: SomeContainerView());
  }
}

class MySubWidgetWayDownTheTree extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder(
      valueListenable: context.read<MyWidgetState>().tabIndexNotifier,
      builder: (_, index, __) => Text("$index"),
    );
  }
}

And this:

class MyWidgetModel extends ChangeNotifier {
  int _index;

  int get index => _index;

  set index(int value) {
    _index = value;
    notifyListeners();
  }
}

class MyWidget extends StatefulWidget {
  @override
  MyWidgetState createState() => MyWidgetState();
}

class MyWidgetState extends State<MyWidget> {
  MyWidgetModel model = MyWidgetModel();

  @override
  Widget build(BuildContext context) {
    return Provider.value(value: this, child: SomeContainerView());
  }
}

class MySubWidgetWayDownTheTree extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Selector<MyWidgetModel, int>(
      selector: (_, model) => model.index,
      builder: (_, index, __) => Text("$index"),
    );
  }
}

I really don't see what is wrong with the first approach or why it would be discouraged in any way. In many cases, State _is_ the model, and it reduces boilerplate to just use it as such.

I won't keep beating the horse though, if you have truly settled on the decision. I do think an extension that wraps a known API, and imposes extra restrictions on it, is a bit of an anti-pattern, and I really don't understand being so opinionated about a very generic tool, but it's your lib :)

[rrousselGit]

You can write what you suggested. It's just that instead of read, use watch.

class MyWidget extends StatefulWidget {
  @override
  MyWidgetState createState() => MyWidgetState();
}

class MyWidgetState extends State<MyWidget> {
  ValueNotifier<int> tabIndexNotifier = ValueNotifier(0);

  @override
  Widget build(BuildContext context) {
    return Provider.value(value: this, child: SomeContainerView());
  }
}

class MySubWidgetWayDownTheTree extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return ValueListenableBuilder(
      valueListenable: context.watch<MyWidgetState>().tabIndexNotifier,
      builder: (_, index, __) => Text("$index"),
    );
  }
}

[esDotDev]

Oh... so in this case _listen=true_ is just effectively ignored?

Did not realize that. Ok fine, you win :p

( mean it's kinda weird cause we're calling it a _watch_, when it's really a _read_, but I digress...)

[rrousselGit]

Oh... so in this case listen=true is just effectively ignored?

Kind of. You are always sending this to the provider, so the object never changes and therefore there's no need to rebuild descendants

[esDotDev]

Just another example of why this rule really should not exist. I have a service, it maps some firebase streams, I want to inject those into some Streambuilders.

Perfectly valid use case, but framework yells at me because it didn't want novice users to learn the difference between read and watch. Which is the one core principle they should understand when using Provider. This decision still really confounds me.

[rrousselGit]

Why wouldn't you use watch here?

[esDotDev]

Well it would be mis-leading, since the service is a singleton-like object that exists for the lifecycle of the app, does not change, and is not a notifier. If someone were to read .watch they might reasonably assume the FirebaseService instance is changed while the app is running or that it is some sort of notifier itself, neither of which is true.

void main(){
  FirebaseService firebase = FirebaseFactory.create();
  runApp(MultiProvider(
    providers: [
      // Firebase
      Provider.value(value: firebase),
    ],
    child: _AppBootstrapper(),
  ));
}

The instance of service is just something I want to pass around, and acts essentially as a factory for various stream configs and CRUD operations so the views can stay very clean:

[rrousselGit]

Widgets shouldn't know whether the value can change or not.

Using read over watch inside build makes refactoring difficult.

The day you change your service to receive an external parameter which can change over time (user id?), you will have a very hard time spotting the fact that you aren't handling updates in your widget

[esDotDev]

Widgets shouldn't know whether the value can change or not.
I can understand this as a general best practice, but imo, at least in this case, its more important that a fellow developer reading the code is not mislead about the core app architecture. Saying read over watch communicates real, useful info, about how the state is flowing.

I think there is no denying that using watch on an item that is never going to change, is mis-leading to read. You can debate which is more important (clear intent vs widget agnostisicm) but I don't think it's self-evident that one should outweigh the other.

Using read over watch inside build makes refactoring difficult. - How so? Just curious.

I'm not sure what you mean with users, we have a setUserId call already on the service, and this is used as a userDoc that underpins all these methods. The service instance still does not need to change.

[rrousselGit]

the service instance still does not need to change.

Many people instead have an immutable service and recreate the service when the parameters change.

[esDotDev]

Ah, ya I totally get that. What I don't get is a generic tool that passes instances up and down a display list, to have some bake-in opinion about my state management, and whether I should use mutable or non-mutable data. Seems like it's not it's role in my codebase.

It's obviously no big deal to work around, but I thought you might appreciate seeing a usecase where this behavior is pretty illogical.