[go: nahoru, domu]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ComputedNotifier streaming class #150143

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

HenriqueNas
Copy link
Contributor

Introduce ComputedNotifier, a new ValueListenable implementation that computes its value based on other ValueListenable instances. This facilitates efficient and reactive updates for derived state, eliminating the need for manual dependency tracking and update propagation.

void main() {
  final valueA = ValueNotifier<int>(5);
  final valueB = ValueNotifier<int>(10);

  /// compute all notifier values
  final computedValue = ComputedNotifier<int>(
    [valueA, valueB],
    () => valueA.value + valueB.value,
  );

  int listenerValue = computedValue.value;
  computedValue.addListener(() => listenerValue = computedValue.value);

  valueA.value = 10;
  print(listenerValue); // 20

  valueB.value = 20;
  print(listenerValue); // 30

  computedValue.dispose();
}

The class listens to changes from provided dependencies and recomputes its value whenever any of them change, notifying its listeners only when the computed value itself changes.

No issue found for this PR

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

`ComputedNotifier` is a new `ValueListenable` implementation that computes its value based on other `ValueListenable` instances. This facilitates efficient and reactive updates for derived state, eliminating the need for manual dependency tracking and update propagation.

The class listens to changes from provided dependencies and recomputes its value whenever any of them change, notifying its listeners only when the computed value itself changes.
Clarifies that `ComputedNotifier` is also a preferred alternative to `Stream` subclasses, aligning with the preference for `ValueNotifier` and `ChangeNotifier` in Flutter framework code.
@github-actions github-actions bot added team Infra upgrades, team productivity, code health, technical debt. See also team: labels. framework flutter/packages/flutter repository. See also f: labels. d: docs/ flutter/flutter/docs, for contributors labels Jun 12, 2024
/// final ValueNotifier<int> b = ValueNotifier<int>(2);
///
/// final ComputedNotifier<int> sum = ComputedNotifier<int>(
/// [a, b, c, d],
Copy link
Member

Choose a reason for hiding this comment

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

Why listen to c and d if only a and b are used in compute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

///
/// Additionally, it is important to dispose of the listeners that are passed
/// to the constructor when they are no longer needed.
///
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@@ -61,13 +61,15 @@ class Counter with ChangeNotifier {
}

void main() {
testWidgets('ChangeNotifier can not dispose in callback', (WidgetTester tester) async {
testWidgets('ChangeNotifier can not dispose in callback',
Copy link
Member

Choose a reason for hiding this comment

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

Can you please undo the formatting changes in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

///
/// final ComputedNotifier<int> sum = ComputedNotifier<int>(
/// [a, b, c, d],
/// () => a.value + b.value,
Copy link
Member

Choose a reason for hiding this comment

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

What worries me about this API design: It's gonna be very easy to use a listenable in the compute function and accidentally forget to include it in the listenable list...

Copy link
Contributor Author
@HenriqueNas HenriqueNas Jun 14, 2024

Choose a reason for hiding this comment

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

You're absolutely right!! But although I think it might happen, I am not very worried about that.. It's a basic programming skill issue, not very related to the API signature.
Also, in some cases, you might not want to include all values ​​used in the computed callback in your listenable list, right?

But, what do you think we can do to improve it? Besides a clear documentation..

Copy link
Contributor Author
@HenriqueNas HenriqueNas Jun 14, 2024

Choose a reason for hiding this comment

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

Maybe we can return the listenable list as a parameter in our callback, so it can reinforces the user to add the listenable to the list:

final ComputedNotifier<String> sum = ComputedNotifier<int>(
  <ValueListenable<int>>[a, b],
  (List<ValueListenable<dynamic>> list) => ((list.first.value as int) + (list[1].value as int)).toString(),
);

To be honest, I do not found this a good solution. Maybe because I don't see it as a ruge problem.

Copy link
Contributor Author
@HenriqueNas HenriqueNas Jun 15, 2024

Choose a reason for hiding this comment

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

You made me intriguer about a solution for this... what do you think about write the API as this?

  final ValueNotifier<int> a = ValueNotifier<int>(1);
  final ValueNotifier<int> b = ValueNotifier<int>(2);

  final ComputedNotifier<int> sum = ComputedNotifier<int>(
      (ValueGetter getter) => getter(a) + getter(b),
  );

  int sumValue = sum.value;
  sum.addListener(() => sumValue = sum.value);

  a.value = 2;
  print(sumValue); // 4

  b.value = 3;
  print(sumValue); // 5

@HenriqueNas
Copy link
Contributor Author

@goderbauer PTAL
...thanks for review it 🫶

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
d: docs/ flutter/flutter/docs, for contributors framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants