[hooks_riverpod] `StateProvider`'s listeners are triggered even when a different State changes

iapicca commented 3 years ago

Describe the bug

StateProvider listeners are triggered even when a different State changes

To Reproduce

run the code sample below
tap any fab

code sample

```dart import 'package:flutter/material.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:hooks_riverpod/all.dart'; final redCounter = StateProvider((ref) => 0); final blueCounter = StateProvider((ref) => 0); void main() { runApp( const ProviderScope( child: MaterialApp(home: MyHomePage()), ), ); } class MyHomePage extends HookWidget { const MyHomePage() : super(key: const ValueKey('MyHomePage')); @override Widget build(BuildContext context) { final red = useProvider(redCounter) ..addListener((_) => print('red is changing')); final blue = useProvider(blueCounter) ..addListener((_) => print('blue is changing')); return Scaffold( body: Row( mainAxisAlignment: MainAxisAlignment.spaceEvenly, children: [ Center(child: MyText(Colors.red, red.state, 'red')), Center(child: MyText(Colors.blue, blue.state, 'blue')), ], ), floatingActionButton: Row( mainAxisAlignment: MainAxisAlignment.spaceEvenly, children: [ FloatingActionButton( backgroundColor: Colors.red, onPressed: () => red.state++), FloatingActionButton( backgroundColor: Colors.blue, onPressed: () => blue.state++), ], ), ); } } class MyText extends StatelessWidget { MyText(this.color, this.counter, String key) : super(key: ValueKey(key)); final Color color; final int counter; @override Widget build(BuildContext context) { print('$key is building'); return Text.rich( TextSpan( text: '$counter', ), style: TextStyle( color: color, )); } } ```

Expected behavior

StateProvider's listener should trigger only when it's state changes

Actual behavior behavior

StateProvider's listener triggers when its state doesn't change if another StateProvider in the same build forces a rebuild

logs

```bash ✓ Built build/app/outputs/flutter-apk/app-debug.apk. I/flutter (22946): red is changing I/flutter (22946): blue is changing I/flutter (22946): MaterialColor(primary value: Color(0xfff44336)) is building I/flutter (22946): MaterialColor(primary value: Color(0xff2196f3)) is building Syncing files to device Pixel 3a... 140ms Flutter run key commands. r Hot reload. 🔥🔥🔥 R Hot restart. h Repeat this help message. d Detach (terminate "flutter run" but leave application running). c Clear the screen q Quit (terminate the application on the device). An Observatory debugger and profiler on Pixel 3a is available at: http://127.0.0.1:34317/gbxJPiaG19o=/ Flutter DevTools, a Flutter debugger and profiler, on Pixel 3a is available at: http://127.0.0.1:9102?uri=http%3A%2F%2F127.0.0.1%3A34317%2FgbxJPiaG19o%3D%2F Running with unsound null safety For more information see https://dart.dev/null-safety/unsound-null-safety Performing hot restart... Restarted application in 1,432ms. I/flutter (22946): red is changing I/flutter (22946): blue is changing I/flutter (22946): [<'red'>] is building I/flutter (22946): [<'blue'>] is building I/flutter (22946): red is changing I/flutter (22946): red is changing I/flutter (22946): blue is changing I/flutter (22946): [<'red'>] is building I/flutter (22946): [<'blue'>] is building ```

pubspec

```yaml name: first_february description: A new Flutter project. version: 1.0.0+1 environment: sdk: ">=2.7.0 <3.0.0" dependencies: flutter: sdk: flutter riverpod: ^0.12.2 hooks_riverpod: ^0.12.2 flutter_hooks: ^0.15.0 dev_dependencies: flutter_test: sdk: flutter mockito: ^4.1.4 flutter: uses-material-design: true ```

doctor

```console [✓] Flutter (Channel master, 1.26.0-18.0.pre.91, on Linux, locale en_US.UTF-8) • Flutter version 1.26.0-18.0.pre.91 at /home/francesco/snap/flutter/common/flutter • Framework revision cb14df5659 (3 hours ago), 2021-02-01 06:17:38 -0500 • Engine revision ce14c8a841 • Dart version 2.12.0 (build 2.12.0-276.0.dev) [✓] Android toolchain - develop for Android devices (Android SDK version 30.0.2) • Android SDK at /home/francesco/Android/Sdk • Platform android-30, build-tools 30.0.2 • ANDROID_SDK_ROOT = /home/francesco/Android/Sdk • Java binary at: /usr/lib/jvm/java-1.8.0-openjdk-amd64/jre/bin/java • Java version OpenJDK Runtime Environment (build 1.8.0_275-8u275-b01-0ubuntu1~20.04-b01) • All Android licenses accepted. [✓] Chrome - develop for the web • CHROME_EXECUTABLE = /snap/bin/chromium [✓] Linux toolchain - develop for Linux desktop • clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final) • cmake version 3.10.2 • ninja version 1.8.2 • pkg-config version 0.29.1 [!] Android Studio (not installed) • Android Studio not found; download from https://developer.android.com/studio/index.html (or visit https://flutter.dev/docs/get-started/install/linux#android-setup for detailed instructions). [✓] Connected device (3 available) • Pixel 3a (mobile) • 965AY0WP5C • android-arm64 • Android 11 (API 30) • Linux (desktop) • linux • linux-x64 • Linux • Chrome (web) • chrome • web-javascript • Chromium 88.0.4324.96 snap ! Doctor found issues in 1 category. francesco@francesco-yoga720:~/projects ```

rrousselGit commented 3 years ago

That's a side-effect of a bug in your code:

    final red = useProvider(redCounter)
      ..addListener((_) => print('red is changing'));

You can't call addListener like that. A new listener would be added whenever the widget rebuild, and the previous listeners are never removed.

The reason why the listener is called in your case is: With StateNotifier.addListener, by default, the new listeners are called immediately (which can be disabled with the fireImmediately parameter) So this is not a bug on Riverpod.

Instead, do:

return ProviderListener<int>(
  provider: redCounter,
  onChange: (context, value) => print('red is changing'),
  child: Scaffold(...),
);

iapicca commented 3 years ago

@rrousselGit would it work if I set the listener inside a hook?

rrousselGit commented 3 years ago

Yes, useEffect is a good candidate

rrousselGit / riverpod