Conversation
Implements the core A2UI v0.9 runtime as a pure-Dart library,
independent of Flutter. Based on the web_core reference implementation.
Includes:
- State models: ComponentModel, SurfaceComponentsModel, SurfaceModel,
SurfaceGroupModel, DataModel (with bubble/cascade notifications and
auto-vivification)
- Reactivity: batch-aware ValueNotifier and ComputedNotifier, built on
top of the shared ChangeNotifier from the listenable layer
- MessageProcessor for all four A2UI message types
- ExpressionParser for ${expression} interpolation
- GenericBinder for schema-driven reactive property resolution
- DataContext/ComponentContext for scoped data access
- Protocol types, CommonSchemas, MinimalCatalog API definitions
- CancellationSignal for async cleanup
Contributes to flutter#828
Contributes to flutter#811
DataPath(['a', 'b']) and DataPath(['a/b']) produced the same hashCode
because segments.join('/') yields "a/b" for both. Per RFC 6901 section
3, these are semantically different pointers ("/a/b" vs "/a~1b").
Use ListEquality from package:collection for both == and hashCode so
each segment is hashed independently.
Run dart fix --apply (183 auto-fixes) and manually resolve all remaining errors and warnings: - Add explicit casts where dart fix removed dynamic annotations, causing type assignment errors (12 errors) - Replace implicit dynamic parameters with Object? throughout (inference_failure_on_untyped_parameter warnings) - Add explicit type arguments to collection literals and constructors - Remove unreachable default clause in Behavior switch - Remove unnecessary cast 4 remaining warnings are for notifyListeners() calls from outside ChangeNotifier subclasses — inherent to the batch() and DataModel force-notify designs, would need an API change to the notifier layer.
batch() and DataModel._getAndNotify() need to trigger notifications from outside a ChangeNotifier subclass. Calling notifyListeners() directly violates @Protected, matching Flutter's design where only the owning subclass decides when to notify. Add a public forceNotify() on our ValueNotifier that delegates to notifyListeners() internally. This gives external coordinators a sanctioned way to trigger notifications — batch() uses it to flush deferred notifications, and DataModel uses it when a container is mutated in place without changing the object reference.
ValueNotifier's setter skips notification when newValue == oldValue. Since onUpdated's value was always `this`, only the null→this transition on the first update ever notified listeners. Use forceNotify() instead.
When GenericBinder rebuilt its bindings (e.g. after a component property update), ComputedNotifiers created by DataContext.resolveListenable for function-call expressions were orphaned without being disposed. These leaked notifiers kept their internal dependency subscriptions active, causing redundant function evaluations on every data model change. The binder now tracks ComputedNotifiers it obtains from resolveListenable and disposes them on rebuild and on dispose.
…otifier GenericBinder's constructor called _resolveInitialProps() (sync, no subscriptions) then immediately connect() which called _rebuildAllBindings() (with subscriptions), fully overwriting the first pass. Removed _resolveInitialProps so resolution happens exactly once. ComputedNotifier's constructor also double-evaluated: super(_compute()) ran the function untracked, then _updateDependencies() ran it again with tracking. Restructured to evaluate once inside a tracker during the super call, then subscribe to the captured dependencies in the body, using a stack to handle reentrant construction safely.
The core library must provide a mechanism to deserialize raw JSON into strongly-typed A2uiMessage objects (per renderer guide Section 3). Dispatches on the discriminator key (createSurface, updateComponents, updateDataModel, deleteSurface) and throws A2uiValidationError for unknown types. Also changes UpdateDataModelMessage.value from dynamic to Object?.
After cancel() fires all listeners, the list was never cleared, keeping closures (and everything they capture) referenced until the signal itself is GC'd. Also adds unit tests for CancellationSignal which previously had no test coverage.
SurfaceGroupModel.addSurface registered an anonymous closure on each surface's onAction but never removed it on deleteSurface. The listener is now stored and explicitly removed before the surface is disposed.
ExpressionParser and _Scanner allocated new RegExp objects on every character check during parsing (_isAlnum, _isDigit, matchesKeyword, skipWhitespace). Replaced with inline codeUnitAt comparisons to avoid unnecessary allocations in this hot path.
Map<String, dynamic> remains for now (Dart JSON convention, would be a larger refactor). All other uses of dynamic (return types, local variables, type parameters on ValueNotifier/ValueListenable, error details fields) are replaced with Object? for static type safety.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the core A2UI protocol implementation for Dart, featuring a reactivity system with computed notifiers, an expression parser for string interpolation, and a central message processor for surface management. Key components include a DataModel for observable state and a GenericBinder for mapping JSON configurations to reactive properties. Feedback identifies critical resource management issues, specifically memory and CPU leaks caused by undisposed nested ComputedNotifiers in DataContext and FormatStringFunction. Additionally, the review highlights a potential stack overflow vulnerability in recursive resolution, a ConcurrentModificationError risk in the cancellation logic, a bug involving shallow schema copying in the MessageProcessor, and non-standard JSON Pointer parsing in the DataPath implementation.
The renderer guide (Section 3) groups catalog API definitions, state models, and context classes together as "The Core Data Layer." The previous split into protocol/ and state/ created a directory-level cycle flagged by layerlens, because these types are inherently co-dependent (FunctionImplementation.execute receives a DataContext, and DataContext invokes catalog functions). The same co-dependency exists in the web_core reference implementation. Merging into core/ reflects the spec's actual layering. The remaining intra-directory file cycles are excluded from layerlens checking since they represent a single cohesive unit per the spec.
If a listener calls removeListener on itself during cancel(), the underlying list is modified during iteration, causing a ConcurrentModificationError. Copy the list and clear it before iterating, matching the pattern used by ChangeNotifier.
_processRefs mutates maps in-place to replace REF: descriptions with $ref pointers. toJsonMap used Map.from (shallow copy), so nested maps were shared with the original Schema.value. This corrupted shared statics like CommonSchemas.dynamicString after the first call to getClientCapabilities. Replaced with a recursive deep copy.
| /// A derived notifier that automatically tracks and listens to other | ||
| /// [ValueListenable] dependencies, recalculating its value only when they | ||
| /// change. | ||
| class ComputedNotifier<T> extends ValueNotifier<T> { |
There was a problem hiding this comment.
So these extensions basically allow creating signal or rx-like chains from ValueNotifier.
For this use case, I'm wondering if there might be other libraries already in use in the Dart ecosystem that could be a good fit. We used rxjs observables for a while on web, but it wasn't as good a fit as signals because it only represents value updates, not really the current value. I wonder if we should consider depending on https://pub.dev/packages/signals ?
There was a problem hiding this comment.
As I know variations of such notifier are copied from package to package.
I expect dependency on signals to slow down dev velocity, because we will have to maintain copy of signals in G3.
There was a problem hiding this comment.
I spent a few hours here, and I now believe it's worth taking on a dependency [on package:signals] here. I kept running into edge cases (or ballooning code complexity) trying to simulate signal model using push-based model like ValueNotifier.
As I know variations of such notifier are copied from package to package.
I did consider a custom (reduced) implementation of package:signals, but even that would be decently complex (version tracking and managing topology of computed values). I also like the idea of using a heavily field-tested library that can also give us things like optimized/eager listener disposal, cycle detection, and graceful handling of edge cases such as a computed evaluation callback creating additional computeds.
I've added package:preact_signals (which package:signals wraps), which is just a Dart port of the same Preact signals used in the TypeScript renderer IIRC. We can upgrade to package:signals if needed when we proceed with migrating package:genui onto package:a2ui_core.
There was a problem hiding this comment.
Side comment: Have you tried getting an agent to compare this API surface to what's inside the existing Gen UI SDK, to try to match up the equivalent classes and see what APIs are changing? It'd be nice to get a sense of how awkward the transition will be. It could make sense to do some renaming / refactoring up front to the Gen UI SDK to reduce the migration diff, e.g. rename any parameters of the Catalog / CatalogItem / Function types etc, so that a2ui_core is more like a drop-in replacement.
| } | ||
|
|
||
| /// A definition of a UI function's API. | ||
| abstract class FunctionApi { |
There was a problem hiding this comment.
I noticed that in Gen UI SDK, we currently have a description parameter on the function too. Is that part of the A2UI standard? I can't remember. We should try to align the A2UI specification and TS and Dart implementations to whatever makes the most sense. Perhaps the description should just be included on the argumentSchema? Or maybe a top-level description is not possible there, so it should be separate.
| // found in the LICENSE file. | ||
|
|
||
| /// A signal that can be used to cancel an operation. | ||
| class CancellationSignal { |
There was a problem hiding this comment.
It feels like we're having to add a lot to ValueNotifier to get the overall signal-like behavior we need. Overall, we need some approach that offers:
- A) Representing the current value and updates to that value
- B) Synchronous processing rather than async (I think!)
- C) Ability to chain transformation functions together, which may depend on multiple inputs
- D) Ability for the ultimate consumer of the output to cancel their subscription or release the reference and have the chain of transformers, instance of function objects etc be destroyed.
Currently, we use ValueNotifier which offers A and B, then we add C and D. I wonder if there is a library out there that offers 3 or 4 of the properties out of the box?
There was a problem hiding this comment.
See #857 (comment). I think we should just use package:preact_signals
| Map<String, dynamic> toJson() => {'id': id, 'basePath': basePath}; | ||
| } | ||
|
|
||
| /// A framework-agnostic engine that transforms raw A2UI JSON payload |
There was a problem hiding this comment.
It is not clear for me what is "payload configurations".
Can you either define this term (in doc comments of some element or in go/genui-glossary ?) or use other words?
|
Thank you for creating good test coverage! |
There was a problem hiding this comment.
Can you rename folder ‘common’ to ‘shared’ or ‘primitives’ as:
- ‘shared’ or ‘primitives’ are used more in dash projects
- ‘common’ is anti-pattern as it is ambiguous: it sometimes mean high level orchestrating helpers and sometimes low level primitives
| /// A derived notifier that automatically tracks and listens to other | ||
| /// [ValueListenable] dependencies, recalculating its value only when they | ||
| /// change. | ||
| class ComputedNotifier<T> extends ValueNotifier<T> { |
There was a problem hiding this comment.
As I know variations of such notifier are copied from package to package.
I expect dependency on signals to slow down dev velocity, because we will have to maintain copy of signals in G3.
| import 'data_model.dart'; | ||
| import 'surface_model.dart'; | ||
|
|
||
| /// A contextual view of the main DataModel. |
There was a problem hiding this comment.
Can you give more details:
- what is contextual view?
- how it is used?
|
|
||
| extension CatalogInvokerExtension on Catalog { | ||
| /// Helper to invoke functions. | ||
| Object? invoker(String name, Map<String, dynamic> args, DataContext context) { |
There was a problem hiding this comment.
Does 'invoker' return function that actually invokes function? Or result of invocation?
If result, should the method be named 'invoke'?
Can you add comment that clarifies this?
Maybe say 'client functions' instead of 'functions', to remove ambiguity?
|
|
||
| DataModel get dataModel => surface.dataModel; | ||
|
|
||
| /// Resolves a path against this context. |
There was a problem hiding this comment.
Can you add more details what 'resolves' mean here?
Will it return absolute path?
May be examples will help.
| return '$base/$relativePath'; | ||
| } | ||
|
|
||
| /// Synchronously evaluates a dynamic value. |
There was a problem hiding this comment.
Can you explain in more details contract of this method?
What is expected for the input?
What will be output for which input?
|
|
||
| /// Fires when a component is removed, providing the ID of the deleted | ||
| /// component. | ||
| ValueListenable<String?> get onDeleted => _onDeleted; |
There was a problem hiding this comment.
onCreated and onDeleted:
ValueListenable is not right type here.
ValueListenable makes sense when there is notion of current value.
Should we use Stream and Stream here?
| } | ||
|
|
||
| /// A template for generating a dynamic list of children. | ||
| class ChildListTemplate { |
There was a problem hiding this comment.
I am missing explanation of what is "template".
As there is no single class where this explanation can be added, should it be added to go/a2ui-glossary ? Or, if there will be many templates, we can create tarent class Template, and add explanation here.
However, I see that this class is not used.
Can you explain target use case in doc comment?
There was a problem hiding this comment.
Templates are an A2UI concept - see https://github.com/google/A2UI/blob/e30a9556cc3a138f6bafbaa6530627ebe8bb1e4a/specification/v0_9/docs/a2ui_protocol.md#collection-scopes-relative-paths
Perhaps they could be in the glossary, though I think the spec explains them well enough.
| } | ||
|
|
||
| /// Triggers a server-side event or a local client-side function. | ||
| class Action { |
There was a problem hiding this comment.
Would it be better to create sealed class Action with two implementations: EventAction and FunctionAction?
There was a problem hiding this comment.
I think this class directly mirrors the structure of the underlying JSON, and that is simpler to understand overall. Though I agree that purely from a Dart perspective, a sealed class is nicer.
DataContext now takes a DataModel + FunctionInvoker callback instead of a SurfaceModel reference. It only ever used surface.dataModel and surface.catalog.invoker, so the full SurfaceModel dependency was unnecessary. ComponentContext still takes a SurfaceModel and wires the pieces together.
ValueNotifier was being used for both stateful values and fire-and-forget events (onCreated, onDeleted, onAction, etc). Events don't have a "current value," and ValueNotifier's equality check silently swallows repeated notifications when the payload is the same reference (e.g. ComponentModel.onUpdated always emitting the same object). EventNotifier is a simple typed callback list with no value storage, modeled after web_core's EventEmitter and the renderer guide's "Event Streams" prerequisite.
The method executes a function and returns the result, so "invoke" is more accurate. The old name read like it returned a function rather than calling one. The tear-off usage (catalog.invoke passed as a FunctionInvoker callback) still reads naturally.
SurfaceModel and SurfaceGroupModel are related but distinct: SurfaceModel is the state for a single surface, SurfaceGroupModel manages multiple surfaces and forwards events between them. Keeping them in separate files makes each easier to find and aligns with the one-class-per-file pattern used by the other models (ComponentModel, DataModel).
Replace vague terms ("payload configurations," "contextual view") with
concrete descriptions of what each class and method actually does.
Add doc comments to resolveSync and resolveListenable clarifying that
one is a one-shot evaluation and the other creates reactive
subscriptions.
"common" is ambiguous about abstraction level. "primitives" is more descriptive: the directory contains foundational data structures and building blocks (data paths, errors, reactivity, cancellation) with no business logic. The restrictive name also discourages dumping higher-level utilities into it as the library grows.
The spec defines a fixed set of return types but we were using raw strings. An enum prevents typos when declaring function implementations and gives IDE autocompletion.
Our custom ComputedNotifier had a fundamental limitation: nested computed values created during evaluation (e.g. by FormatStringFunction or custom functions calling resolveListenable) leaked subscriptions because the push-based addListener/removeListener model can't track and dispose children created during re-evaluation. This is the core problem that signals libraries solve with pull-based lazy evaluation. preact_signals is a Dart port of Preact's signal library. It provides signal, computed, batch, and effect with correct nested disposal, diamond dependency deduplication, and lazy evaluation. 60KB, pure Dart, only depends on meta. reactivity.dart now re-exports preact_signals primitives instead of defining custom classes. The GenericBinder no longer needs to manually track and dispose ComputedNotifiers. The listenable/ layer (Flutter's ChangeNotifier replica) is retained for Phase 3 Flutter integration but is no longer used by a2ui_core itself.
| } | ||
|
|
||
| /// Triggers a server-side event or a local client-side function. | ||
| class Action { |
There was a problem hiding this comment.
I think this class directly mirrors the structure of the underlying JSON, and that is simpler to understand overall. Though I agree that purely from a Dart perspective, a sealed class is nicer.
| } | ||
|
|
||
| /// A template for generating a dynamic list of children. | ||
| class ChildListTemplate { |
There was a problem hiding this comment.
Templates are an A2UI concept - see https://github.com/google/A2UI/blob/e30a9556cc3a138f6bafbaa6530627ebe8bb1e4a/specification/v0_9/docs/a2ui_protocol.md#collection-scopes-relative-paths
Perhaps they could be in the glossary, though I think the spec explains them well enough.
|
|
||
| /// Returns a reactive signal that re-evaluates a dynamic value | ||
| /// whenever its underlying data dependencies change. | ||
| ReadonlySignal<Object?> resolveListenable(Object? value) { |
| BehaviorNode(this.type, {this.shape, this.element}); | ||
| } | ||
|
|
||
| class ChildNode { |
There was a problem hiding this comment.
Maybe ChildNodeRef might be a better name? In future we might introduce the concept of actual nodes which contain the resolved props etc for the node.
3 participants
Resolves #828
Builds on #831 toward objective 3.1 as described in #811: establishing a framework-agnostic state engine for the v0.9 A2UI spec (tracking issue: #828). See individual commit messages for adjustments/fixes made on top of #831.
The diff is large, but every file is interdependent and I didn't see any productive way to split up the change into smaller ones. I tried to match the structure of the renderer guide in the code, which should make it possible to individually check each module against the corresponding section in the guide.
Resources:
web_coreimplementation, which can be used as a reference to compare this one with: https://github.com/google/A2UI/tree/main/renderers/web_coreKnown gaps left out of scope of this PR
addLifecycleListeneronMessageProcessor. Currently implemented using a different API shape (readingSurfaceGroupModeldirectly). To be re-evaluated.Some observations on the renderer guide/a2ui spec (to be considered later)
Next steps after this is merged
Per #828,
a2ui_coreintopackages/genuiSurfaceControllerto useMessageProcessorComponentContextPre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-devrel channel on Discord.