Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/a2ui_core/lib/a2ui_core.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,9 @@
export 'src/listenable/error_reporting.dart'
show ListenableError, ListenableErrorDetails;
export 'src/listenable/notifiers.dart'
show ChangeNotifier, GenUiListenable, GenUiValueListenable, ValueNotifier;
show
GenUiChangeNotifier,
GenUiListenable,
GenUiValueListenable,
GenUiValueNotifier;
export 'src/listenable/primitives.dart' show VoidCallback;
24 changes: 12 additions & 12 deletions packages/a2ui_core/lib/src/listenable/notifiers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ abstract class GenUiValueListenable<T> extends GenUiListenable {
///
/// This class should not be modified, because it is temporary and should be
/// replaced with dash-wide alternative.
mixin class ChangeNotifier implements GenUiListenable {
mixin class GenUiChangeNotifier implements GenUiListenable {
int _count = 0;
// The _listeners is intentionally set to a fixed-length _GrowableList instead
// of const [].
Expand All @@ -84,8 +84,8 @@ mixin class ChangeNotifier implements GenUiListenable {
int _reentrantlyRemovedListeners = 0;
bool _debugDisposed = false;

/// Used by subclasses to assert that the [ChangeNotifier] has not yet been
/// disposed.
/// Used by subclasses to assert that the [GenUiChangeNotifier] has not
/// yet been disposed.
///
/// {@tool snippet}
/// The [debugAssertNotDisposed] function should only be called inside of an
Expand All @@ -103,7 +103,7 @@ mixin class ChangeNotifier implements GenUiListenable {
// This is static and not an instance method because too many people try to
// implement ChangeNotifier instead of extending it (and so it is too breaking
// to add a method, especially for debug).
static bool debugAssertNotDisposed(ChangeNotifier notifier) {
static bool debugAssertNotDisposed(GenUiChangeNotifier notifier) {
assert(() {
if (notifier._debugDisposed) {
throw ListenableErrorReporting.createError(
Expand Down Expand Up @@ -150,7 +150,7 @@ mixin class ChangeNotifier implements GenUiListenable {
/// (e.g. in response to a notification), it will still be called again. If,
/// on the other hand, it is removed as many times as it was registered, then
/// it will no longer be called. This odd behavior is the result of the
/// [ChangeNotifier] not being able to determine which listener is being
/// [GenUiChangeNotifier] not being able to determine which listener is being
/// removed, since they are identical, therefore it will conservatively still
/// call all the listeners when it knows that any are still registered.
///
Expand All @@ -165,7 +165,7 @@ mixin class ChangeNotifier implements GenUiListenable {
/// the list of closures that are notified when the object changes.
@override
void addListener(VoidCallback listener) {
assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(GenUiChangeNotifier.debugAssertNotDisposed(this));

if (_count == _listeners.length) {
if (_count == 0) {
Expand Down Expand Up @@ -269,7 +269,7 @@ mixin class ChangeNotifier implements GenUiListenable {
/// listeners or not immediately before disposal.
@mustCallSuper
void dispose() {
assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(GenUiChangeNotifier.debugAssertNotDisposed(this));
if (_notificationCallStackDepth > 0) {
throw ListenableErrorReporting.createError(
'The "dispose()" method on $this was called during the call to '
Expand Down Expand Up @@ -304,7 +304,7 @@ mixin class ChangeNotifier implements GenUiListenable {
@visibleForTesting
@pragma('vm:notify-debugger-on-exception')
void notifyListeners() {
assert(ChangeNotifier.debugAssertNotDisposed(this));
assert(GenUiChangeNotifier.debugAssertNotDisposed(this));
if (_count == 0) {
return;
}
Expand Down Expand Up @@ -403,16 +403,16 @@ class _MergingListenable extends GenUiListenable {
}
}

/// A [ChangeNotifier] that holds a single value.
/// A [GenUiChangeNotifier] that holds a single value.
///
/// Dart replica of Flutter's [ValueNotifier](https://api.flutter.dev/flutter/foundation/ValueNotifier-class.html)
///
/// This class should not be modified, because it is temporary and should be
/// replaced with dash-wide alternative.
class ValueNotifier<T> extends ChangeNotifier
class GenUiValueNotifier<T> extends GenUiChangeNotifier
implements GenUiValueListenable<T> {
/// Creates a [ChangeNotifier] that wraps this value.
ValueNotifier(this._value);
/// Creates a [GenUiChangeNotifier] that wraps this value.
GenUiValueNotifier(this._value);
Comment on lines +406 to +415
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The doc comments for GenUiValueNotifier and its constructor should refer to GenUiValueNotifier specifically rather than the base class GenUiChangeNotifier to provide clearer documentation for users of this class.

/// A [GenUiValueNotifier] that holds a single value.
///
/// Dart replica of Flutter's [ValueNotifier](https://api.flutter.dev/flutter/foundation/ValueNotifier-class.html)
///
/// This class should not be modified, because it is temporary and should be
/// replaced with dash-wide alternative.
class GenUiValueNotifier<T> extends GenUiChangeNotifier
    implements GenUiValueListenable<T> {
  /// Creates a [GenUiValueNotifier] that wraps this value.
  GenUiValueNotifier(this._value);


/// The current value stored in this notifier.
///
Expand Down
14 changes: 7 additions & 7 deletions packages/a2ui_core/test/listenable/listenable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import 'package:a2ui_core/a2ui_core.dart';
import 'package:a2ui_core/src/listenable/error_reporting.dart';
import 'package:test/test.dart';

class TestNotifier extends ChangeNotifier {
class TestNotifier extends GenUiChangeNotifier {
void notify() {
notifyListeners();
}

bool get isListenedTo => hasListeners;
}

class HasListenersTester<T> extends ValueNotifier<T> {
class HasListenersTester<T> extends GenUiValueNotifier<T> {
HasListenersTester(super.value);
bool get testHasListeners => hasListeners;
}
Expand All @@ -26,7 +26,7 @@ class A {
}
}

class B extends A with ChangeNotifier {
class B extends A with GenUiChangeNotifier {
B();

@override
Expand All @@ -36,7 +36,7 @@ class B extends A with ChangeNotifier {
}
}

class Counter with ChangeNotifier {
class Counter with GenUiChangeNotifier {
Counter();

int get value => _value;
Expand Down Expand Up @@ -377,7 +377,7 @@ void main() {
});

test('Value notifier', () {
final notifier = ValueNotifier<double>(2.0);
final notifier = GenUiValueNotifier<double>(2.0);

final log = <double>[];
void listener() {
Expand Down Expand Up @@ -512,11 +512,11 @@ void main() {

test('Calling debugAssertNotDisposed works as intended', () {
final testNotifier = TestNotifier();
expect(ChangeNotifier.debugAssertNotDisposed(testNotifier), isTrue);
expect(GenUiChangeNotifier.debugAssertNotDisposed(testNotifier), isTrue);
testNotifier.dispose();
ListenableError? error;
try {
ChangeNotifier.debugAssertNotDisposed(testNotifier);
GenUiChangeNotifier.debugAssertNotDisposed(testNotifier);
// ignore: avoid_catching_errors
} on ListenableError catch (e) {
error = e;
Expand Down
12 changes: 6 additions & 6 deletions packages/a2ui_core/test/listenable/smoke_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'package:a2ui_core/a2ui_core.dart';
import 'package:test/test.dart';

// ignore: unused_element, tests that ValueNotifier can be implemented.
class _ValueNotifierImplementation<T> implements ValueNotifier<T> {
class _ValueNotifierImplementation<T> implements GenUiValueNotifier<T> {
@override
void addListener(VoidCallback listener) {}

Expand All @@ -30,12 +30,12 @@ class _ValueNotifierImplementation<T> implements ValueNotifier<T> {
}

// ignore: unused_element, tests that ValueNotifier can be extended.
class _ValueNotifierExtension<T> extends ValueNotifier<T> {
class _ValueNotifierExtension<T> extends GenUiValueNotifier<T> {
_ValueNotifierExtension(super.value);
}

// ignore: unused_element, tests that ChangeNotifier can be implemented.
class _ChangeNotifierImplementation implements ChangeNotifier {
class _ChangeNotifierImplementation implements GenUiChangeNotifier {
@override
void addListener(VoidCallback listener) {}

Expand All @@ -53,11 +53,11 @@ class _ChangeNotifierImplementation implements ChangeNotifier {
}

// ignore: unused_element, tests that ChangeNotifier can be extended.
class _ChangeNotifierExtention extends ChangeNotifier {}
class _ChangeNotifierExtention extends GenUiChangeNotifier {}

void main() {
test('ValueNotifier basic functionality is working', () {
final ValueNotifier<int> notifier = ValueNotifier(1);
final GenUiValueNotifier<int> notifier = GenUiValueNotifier(1);
addTearDown(notifier.dispose);
var count = 0;
notifier.addListener(() => count++);
Expand All @@ -72,7 +72,7 @@ void main() {
});

test('ChangeNotifier basic functionality is working', () {
final notifier = ChangeNotifier();
final notifier = GenUiChangeNotifier();
addTearDown(notifier.dispose);
var count = 0;
notifier.addListener(() => count++);
Expand Down
50 changes: 50 additions & 0 deletions packages/genui/lib/src/primitives/flutter_listenable.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright 2025 The Flutter Authors.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:a2ui_core/a2ui_core.dart';
import 'package:flutter/foundation.dart';

/// Extension to convert [GenUiListenable] to [Listenable].
///
/// Enables using [GenUiListenable] with Flutter widgets
/// that accept [Listenable].
extension FlutterListenable on GenUiListenable {
Comment on lines +8 to +12
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Creating a new FlutterListenableAdapter on every call to listenable() can lead to performance issues in Flutter. Widgets like ListenableBuilder use identity checks to manage subscriptions; if a new adapter is provided on every rebuild, it will cause unnecessary listener churn. Consider caching the adapter using an Expando.

Suggested change
/// Extension to convert [GenUiListenable] to [Listenable].
///
/// Enables using [GenUiListenable] with Flutter widgets
/// that accept [Listenable].
extension FlutterListenable on GenUiListenable {
extension FlutterListenable on GenUiListenable {
static final _adapters = Expando<Listenable>();
Listenable listenable() {
return _adapters[this] ??= FlutterListenableAdapter(this);
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

flutter build creates a lot of classes on every call.

if class is lightweight (like this adapter) it is ok, it will be easily garbage collected.

Listenable listenable() {
return FlutterListenableAdapter(this);
}
}

/// Extensions to convert GenUi value listenables to Flutter value listenables.
extension GenUiValueListenableFlutterExtension<T> on GenUiValueListenable<T> {
/// Converts this [GenUiValueListenable] to a Flutter [ValueListenable].
ValueListenable<T> valueListenable() =>
FlutterValueListenableAdapter<T>(this);
}

class FlutterListenableAdapter implements Listenable {
FlutterListenableAdapter(this._listenable);

final GenUiListenable _listenable;

@override
void addListener(VoidCallback listener) {
_listenable.addListener(listener);
}

@override
void removeListener(VoidCallback listener) {
_listenable.removeListener(listener);
}
}
Comment on lines +5 to +39
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current implementation of the Flutter adapter has several issues that need to be addressed:

  1. Ambiguous Import: Both package:a2ui_core/a2ui_core.dart and package:flutter/foundation.dart export the name VoidCallback. This will cause a compilation error. You should hide VoidCallback from the a2ui_core import.
  2. Missing ValueListenable Support: The PR aims to adapt GenUiValueNotifier, but the current adapter only implements Listenable. To fully support GenUiValueNotifier (and other GenUiValueListenable types) in Flutter, the adapter should also implement ValueListenable<T>. This allows it to be used with widgets like ValueListenableBuilder.
  3. Encapsulation: The adapter classes are implementation details and should be private (prefixed with _) to avoid polluting the public API.
  4. Naming: The extension name FlutterListenable is a bit generic; using a more descriptive name like GenUiListenableFlutterExtension is preferred.
import 'package:a2ui_core/a2ui_core.dart' hide VoidCallback;
import 'package:flutter/foundation.dart';

/// Extensions to convert GenUi listenables to Flutter listenables.
extension GenUiListenableFlutterExtension on GenUiListenable {
  /// Converts this [GenUiListenable] to a Flutter [Listenable].
  Listenable toFlutterListenable() => _FlutterListenableAdapter(this);
}

/// Extensions to convert GenUi value listenables to Flutter value listenables.
extension GenUiValueListenableFlutterExtension<T> on GenUiValueListenable<T> {
  /// Converts this [GenUiValueListenable] to a Flutter [ValueListenable].
  ValueListenable<T> toFlutterValueListenable() =>
      _FlutterValueListenableAdapter<T>(this);
}

class _FlutterListenableAdapter implements Listenable {
  _FlutterListenableAdapter(this._listenable);

  final GenUiListenable _listenable;

  @override
  void addListener(VoidCallback listener) => _listenable.addListener(listener);

  @override
  void removeListener(VoidCallback listener) =>
      _listenable.removeListener(listener);
}

class _FlutterValueListenableAdapter<T> extends _FlutterListenableAdapter
    implements ValueListenable<T> {
  _FlutterValueListenableAdapter(GenUiValueListenable<T> listenable)
      : super(listenable);

  GenUiValueListenable<T> get _valueListenable =>
      _listenable as GenUiValueListenable<T>;

  @override
  T get value => _valueListenable.value;
}


class FlutterValueListenableAdapter<T> extends FlutterListenableAdapter
implements ValueListenable<T> {
FlutterValueListenableAdapter(GenUiValueListenable<T> super.listenable);

GenUiValueListenable<T> get _valueListenable =>
_listenable as GenUiValueListenable<T>;

@override
T get value => _valueListenable.value;
}
1 change: 1 addition & 0 deletions packages/genui/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ environment:
resolution: workspace

dependencies:
a2ui_core: ^0.0.1-dev001
audioplayers: ^6.6.0
collection: ^1.19.1
flutter:
Expand Down
77 changes: 77 additions & 0 deletions packages/genui/test/primitives/flutter_listenable_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2025 The Flutter Authors.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:a2ui_core/a2ui_core.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:genui/src/primitives/flutter_listenable.dart';

class TestGenUiListenable extends GenUiListenable {
int addListenerCount = 0;
int removeListenerCount = 0;
VoidCallback? lastAddedListener;
VoidCallback? lastRemovedListener;

@override
void addListener(VoidCallback listener) {
addListenerCount++;
lastAddedListener = listener;
}

@override
void removeListener(VoidCallback listener) {
removeListenerCount++;
lastRemovedListener = listener;
}
}

class TestGenUiValueListenable<T> extends TestGenUiListenable
implements GenUiValueListenable<T> {
TestGenUiValueListenable(this.value);

@override
T value;
}

void main() {
group('FlutterListenable', () {
test('adapter registers and unregisters listeners correctly', () {
final listenable = TestGenUiListenable();
final Listenable adapter = listenable.listenable();

expect(adapter, isA<Listenable>());
expect(adapter, isA<FlutterListenableAdapter>());

void listener() {}

adapter.addListener(listener);
expect(listenable.addListenerCount, 1);
expect(listenable.lastAddedListener, listener);

adapter.removeListener(listener);
expect(listenable.removeListenerCount, 1);
expect(listenable.lastRemovedListener, listener);
});

test('valueListenable adapter works correctly', () {
final valueListenable = TestGenUiValueListenable<int>(42);
final ValueListenable<int> adapter = valueListenable.valueListenable();

expect(adapter, isA<ValueListenable<int>>());
expect(adapter, isA<FlutterValueListenableAdapter<int>>());

expect(adapter.value, 42);

void listener() {}

adapter.addListener(listener);
expect(valueListenable.addListenerCount, 1);
expect(valueListenable.lastAddedListener, listener);

adapter.removeListener(listener);
expect(valueListenable.removeListenerCount, 1);
expect(valueListenable.lastRemovedListener, listener);
});
});
}
Loading