Skip to content

Revert "Create Flutter adapter for GenUiValueNotifier. (#860)"#891

Merged
jacobsimionato merged 16 commits into
flutter:mainfrom
jacobsimionato:revert-860
Apr 27, 2026
Merged

Revert "Create Flutter adapter for GenUiValueNotifier. (#860)"#891
jacobsimionato merged 16 commits into
flutter:mainfrom
jacobsimionato:revert-860

Conversation

@jacobsimionato
Copy link
Copy Markdown
Collaborator

@jacobsimionato jacobsimionato commented Apr 27, 2026

Reverts #860

This removes the dependency of genui on a2ui_core so we can release genui without releasing a2ui_core.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request renames GenUiChangeNotifier and GenUiValueNotifier to ChangeNotifier and ValueNotifier, updates associated tests, and removes the export of these notifiers from the core library. It also removes the a2ui_core dependency from the genui package and deletes the Flutter listenable adapter. The review feedback highlights that removing the export is a breaking change that restricts access to core interfaces. Furthermore, renaming the classes to ChangeNotifier and ValueNotifier is discouraged due to likely naming collisions with Flutter's foundation library and inconsistency with the GenUiListenable interface.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/a2ui_core/lib/a2ui_core.dart (21-22)

high

The removal of the export for src/listenable/notifiers.dart makes GenUiListenable and GenUiValueListenable private to the package. If these interfaces are intended to be part of the core protocol used by other packages (like genui or external consumers), they should remain exported. If the intention is to deprecate them in favor of the new reactivity primitives, consider adding a deprecation notice instead of removing the export entirely, which is a breaking change.

packages/a2ui_core/lib/src/listenable/notifiers.dart (68)

medium

Renaming GenUiChangeNotifier to ChangeNotifier creates a naming inconsistency with the interface it implements (GenUiListenable). Additionally, using the name ChangeNotifier in a package within the Flutter ecosystem is highly likely to cause name collisions with package:flutter/foundation.dart. It is recommended to retain the GenUi prefix to ensure clarity and avoid ambiguity for consumers of this package.

mixin class GenUiChangeNotifier implements GenUiListenable {

packages/a2ui_core/lib/src/listenable/notifiers.dart (412)

medium

Similar to ChangeNotifier, renaming GenUiValueNotifier to ValueNotifier introduces an inconsistency with GenUiValueListenable and increases the risk of name collisions with Flutter's ValueNotifier. Maintaining the GenUi prefix is safer and more consistent with the rest of the listenable API in this file.

class GenUiValueNotifier<T> extends GenUiChangeNotifier

@jacobsimionato jacobsimionato requested a review from polina-c April 27, 2026 03:53
@jacobsimionato jacobsimionato merged commit 5071a03 into flutter:main Apr 27, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants