Provider: Only the first instance returned by builder of ProxyProvider (ChangeNotifierProxyProvider) is propagated down the tree

Created on 4 Jul 2019  路  11Comments  路  Source: rrousselGit/provider

Expected behaviour:

  1. Every time the floating action button is pushed, DecCounter's decimal value is incremented.
  2. DecCounter extends ValueNotifier, so the change of value causes the builder of ListenableProxyProvider<DecCounter, HexCounter>() to be called.
  3. In the builder, HexCounter, which also extends ValueNotifier, is newly instantiated with the decimal value passed in.
  4. The value is converted to a hexadecimal number in HexCounter.
  5. The change is notified and an update of Text should be triggered.
class DecCounter extends ValueNotifier<int> {
  DecCounter() : super(0);

  void increment() => value++;
}

class HexCounter extends ValueNotifier<String> {
  HexCounter(int decimal) : super(decimal.toRadixString(16));
}
import 'package:flutter/material.dart';
import 'package:provider/provider.dart';

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

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: MultiProvider(
        providers: [
          ListenableProvider<DecCounter>(
            builder: (_) => DecCounter(),
            dispose: (_, decCounter) => decCounter.dispose(),
          ),
          ListenableProxyProvider<DecCounter, HexCounter>(
            builder: (_, decCounter, __) => HexCounter(decCounter.value),
            dispose: (_, hexCounter) => hexCounter.dispose(),
          ),
        ],
        child: Scaffold(
          body: _CounterView(),
          floatingActionButton: Consumer<DecCounter>(
            builder: (_, counter, __) {
              return FloatingActionButton(
                onPressed: counter.increment,
                child: const Icon(Icons.add),
              );
            },
          ),
        ),
      ),
    );
  }
}

class _CounterView extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    final counter = Provider.of<HexCounter>(context);

    return Center(
      child: Text(counter.value.toString()),
    );
  }
}

This code does not work as I expected. The number on the screen is not incremented.

I tried many changes in my code and managed to get it to work finally by using and returning the existing instance of HexCounter that is obtained from the last parameter of the builder.

 class HexCounter extends ValueNotifier<String> {
-  HexCounter(int decimal) : super(decimal.toRadixString(16));
+  HexCounter() : super('0');
+
+  void setValue(int decimal) {
+    value = decimal.toRadixString(16);
+  }
 }
 ListenableProxyProvider<DecCounter, HexCounter>(
-  builder: (_, decCounter, __) => HexCounter(decCounter.value),
+  initialBuilder: (_) => HexCounter(),
+  builder: (_, decCounter, prevHexCounter) {
+    return prevHexCounter..setValue(decCounter.value);
+  },
   dispose: (_, hexCounter) => hexCounter.dispose(),
 ),

It looks like returning different HexCounter instances by the builder stops propagation. Am I right in thinking so? Does the builder of ProxyProvider / ChangeNotifierProxyProvider / ListenableProxyProvider have to return the same instance on every rebuild?

If so, it seems the documentation of ProxyProvider's constructor may need a little more detail to prevent misuse.

As opposed to the builder parameter of Provider, builder may be called more than once.

This misled me to think the builder was allowed to return a new instance every time.

bug documentation

Most helpful comment

A few things

  • ValueNotifier is a ChangeNotifier. You probably want ChangeNotifierProvider instead (for the auto dispose)
  • Changing the instance of your listenable subclass should be supported. That's a bug
  • You probably don't want your HexCounter to extend ValueNotifier since it's immutable.
    And for immutable data, you can just use ProxyProvider, which is not bugged.

Here's the fixed code:

class HexCounter {
  HexCounter(int decimal) : value = decimal.toRadixString(16);

  final String value;
}

MultiProvider(
  providers: [
          ChangeNotifierProvider(builder: (_) => DecCounter()),
          ProxyProvider<DecCounter, HexCounter>(
            builder: (_, decCounter, __) => HexCounter(decCounter.value),
          ),
  ]
  child: ...

All 11 comments

A few things

  • ValueNotifier is a ChangeNotifier. You probably want ChangeNotifierProvider instead (for the auto dispose)
  • Changing the instance of your listenable subclass should be supported. That's a bug
  • You probably don't want your HexCounter to extend ValueNotifier since it's immutable.
    And for immutable data, you can just use ProxyProvider, which is not bugged.

Here's the fixed code:

class HexCounter {
  HexCounter(int decimal) : value = decimal.toRadixString(16);

  final String value;
}

MultiProvider(
  providers: [
          ChangeNotifierProvider(builder: (_) => DecCounter()),
          ProxyProvider<DecCounter, HexCounter>(
            builder: (_, decCounter, __) => HexCounter(decCounter.value),
          ),
  ]
  child: ...

Wow, quick response!

Thanks you for the great info.
I had some misunderstandings. I think I need to update my article about this...

This is a really useful package. Keep up the good work 馃槈

Thanks!

About that bug, I'll try to fix it this week-end. It'll be released along with the 3.1.0

Just a quick update on that one:
After some thinking, I've realized that it might be more logical to "purposefully not support that".

It's currently very easy to do:

ChangeNotifierProxyProvider<A, B>(
  builder: (_, a, __) => B(a),
)

or:

ChangeNotifierProxyProvider<A, B>(
  builder: (_, a, __) => a.someNotifier,
)

when the only legitimate usage is:

ChangeNotifierProxyProvider<A, B>(
  initialBuilder: (_) => B(),
  builder: (_, a, b) => b..a = a,
)

because in the first example we'll lose the state of our ChangeNotifier on A update.

It might be more logical to mark initialBuilder as @required and to expect that the instance never changes.

I think in that case the initial builder function should have an A in it's type signature, no?

Further renaming the parameter name of the return type from prev to something such as instance would make sense.

I think in that case the initial builder function should have an A in it's type signature, no?

No, because the initialBuilder will never ever be called again. So having it receive parameters that can change over time is relatively dangerous.

Further renaming the parameter name of the return type from prev to something such as instance would make sense.

You can use whatever name you like 馃槃

Perhaps I'm abusing the API (I'm new to flutter in general so apologies and thank you for your advice). I have been using this for a class which has an A, and always needs an A, but the provided A may change over time. In this case, not having it in the initial builder, even though it's available and built in the tree the first time build is called, is slightly confusing.

How could I use the API differently to achieve a similar effect? Or am I thinking of this problem space wrong?

It's hard to answer like that. Depending on what you're doing, you may not even need your class to be a ChangeNotifier.

I'll need more information.
Consider asking that on StackOverflow instead

It might be more logical to mark initialBuilder as @required and to expect that the instance never changes.

It sounds like a simpler and better solution. DI via constructor like B(a) becomes impossible, but it is trivial.

It is probably even better not only to make such a change but also to have it documented.

Is the change going to be applied to the other two types of proxy providers too? If not so, it may cause another confusion.

For ProxyProvider, no, it won't get such change.
It's used with immutable data, and updating the value is just changing the object. It doesn't really use initialBuilder actually.

it is probably even better not only to make such a change but also to have it documented.

Indeed. In any case, it'd throw an exception with an error message that explains what happened and what we should do instead.

Ah, you're right!
There is not much point in making initialBuilder required for ProxyProvider.
Thanks for the explanation.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Alfie-AD picture Alfie-AD  路  4Comments

jihongboo picture jihongboo  路  4Comments

arnoutvandervorst picture arnoutvandervorst  路  4Comments

mhd-barikhan picture mhd-barikhan  路  5Comments

guopeng1994 picture guopeng1994  路  4Comments