diff --git a/CHANGELOG.md b/CHANGELOG.md index a88b125..ab9393e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ All notable changes to this project will be documented in this file. ### Added +- **`MagicSelector` rebuilds one subtree when one part of a controller changes.** `refreshUI()` notifies every listener and `MagicStatefulViewState` answers with `setState` on the whole view, which is the right default and stops being cheap on a screen where one field changes often and most of the screen does not care: a consumer measured one keystroke in a search field rebuilding 220 styled containers. `MagicBuilder` could not help, because it needs a `ValueListenable` and a controller is a `ChangeNotifier`. The selector caches the widget its builder returned and, while the selected value compares equal, returns that same instance, so `Element.updateChild` short circuits on `child.widget == newWidget` and never descends. Returning an identical instance rather than skipping a `setState` is what makes it work under a parent that rebuilds anyway. Two rules follow: `builder` must be a pure function of the selected value (select a record to watch several fields), and equality is plain `==`, so a selector returning a freshly built `List` never matches its own cache. Deep comparison is deliberately not used, because walking a ten thousand element list per keystroke costs more than the rebuild it prevents. (`lib/src/ui/magic_selector.dart`) + - **`MagicPaginator.isRefreshing` and `.isLoadingMore`, because a list has three loading states and one flag cannot carry them.** A first load shows a skeleton, a refresh keeps the rows the reader is already looking at, and a next page puts a footer under the last row. Read off `isLoading` alone the second and third are indistinguishable, so a screen either blanks itself on every filter change or grows a footer promising a page nothing asked for. Both are false on a first load (nothing on screen to preserve, nothing being appended) and all three are false once the request lands. The distinction only exists DURING a request, which is why `_isReset` is set beside `_isLoading` and before the notification rather than derived afterwards: by the time a caller can await the future there is nothing left to tell apart. One window is documented rather than changed: a `refresh()` deferred behind an in-flight `loadMore()` keeps reporting `isLoadingMore` until that page lands, which is what is happening on the wire and the only path where the flags follow the request rather than the caller's most recent ask. (`lib/src/http/magic_paginator.dart`) - **`MagicPaginator.total`, read from `meta.total`.** The size of the collection rather than of the pages in hand: `items.length` answers "how much have I fetched", and a header reading "11 of 240" needs the other number, which a consumer previously had to fetch a second time or parse out of a response this class had already parsed. Null on a cursor collection, because Laravel's `cursorPaginate()` deliberately does not count and a total invented from the loaded page would be wrong rather than approximate. Read with `containsKey` before the mode branches, so a page that says nothing about the count leaves the last known value alone: an endpoint sending the total on page one only would otherwise have it erased by page two. Cleared on a reset, since a reset is usually a different question and the previous count describes a collection that no longer exists. (`lib/src/http/magic_paginator.dart`) diff --git a/doc/basics/ui-helpers.md b/doc/basics/ui-helpers.md index 640a225..af8c922 100644 --- a/doc/basics/ui-helpers.md +++ b/doc/basics/ui-helpers.md @@ -12,6 +12,7 @@ Magic provides context-free UI feedback utilities, reactive widget builders, dec - [Toast Messages](#toast-messages) - [Configuration](#configuration) - [MagicBuilder](#magic-builder) +- [MagicSelector](#magic-selector) - [MagicTitle](#magic-title) - [MagicResponsiveView](#magic-responsive-view) - [Extended Breakpoints](#extended-breakpoints) @@ -350,6 +351,89 @@ class MonitorShowView extends MagicStatefulView { > [!TIP] > For E2E drivability, prefer `MagicBuilder` over `setState` on the parent widget. Targeted subtree rebuilds keep interactive element identity stable so dusk agents do not lose their references mid-action. + +## MagicSelector + +`MagicSelector` rebuilds one subtree when one part of a controller changes, and leaves it alone the rest of the time. Reach for it when `MagicBuilder` cannot help, which is whenever the thing you want to watch is a plain field on a `MagicController` rather than a `ValueListenable`. + +```dart +MagicSelector( + controller: controller, + selector: (GuideController c) => c.countLabel, + builder: (String label) => Text(label), +) +``` + +### What it is for + +`refreshUI()` notifies every listener, and `MagicStatefulViewState` answers by calling `setState` on the whole view. That is the right default: a controller does not know which of its fields a screen reads, and a view that rebuilds is always correct. + +It stops being cheap on a screen where one field changes often and most of the screen does not care. A search field is the worked example. Every keystroke is a notification, and one keystroke on a real screen was measured rebuilding 220 styled containers, almost none of which could have looked different. + +### How it avoids the rebuild + +It caches the widget the builder returned and, while the selected value compares equal, returns that same **instance**. `Element.updateChild` short circuits when the new widget is `==` to the mounted one, so an identical instance ends the descent there and the subtree is never visited. + +That is what makes it work under a parent that rebuilds anyway. A widget that merely skipped its own `setState` would still be rebuilt from above, which is the situation inside every `MagicStatefulView`. + +### The contract + +`builder` must be a pure function of the value it is handed. A cached child cannot see anything else the closure captured: + +```dart +// WRONG: `total` is captured and nothing here watches it, so the line reads +// a stale total for as long as `count` happens not to move. +MagicSelector( + controller: c, + selector: (C c) => c.count, + builder: (int count) => Text('$count of $total'), +) +``` + +Select both instead. A Dart record has value equality, so it compares by content and the cache still holds: + +```dart +MagicSelector( + controller: c, + selector: (C c) => (c.count, c.total), + builder: ((int, int) v) => Text('${v.$1} of ${v.$2}'), +) +``` + +Reading an `InheritedWidget` inside the cached subtree needs no selection. `Theme.of`, `MediaQuery.of` and `WindTheme.of` register their own dependency, and the framework rebuilds a dependent element directly rather than through its parent. + +The word doing the work there is **inside**. A lookup written in the enclosing `build` and captured by the closure is the captured-`total` hole wearing different clothes, and a dark-mode toggle is a likelier way to meet it: + +```dart +// WRONG: `context` belongs to the view's build, so a theme change rebuilds the +// view, the cache is served, and this subtree keeps the old theme. +MagicSelector( + controller: c, + selector: (C c) => c.count, + builder: (int n) => WDiv(className: WindTheme.of(context).surface), +) + +// Right: the lookup happens inside the built subtree, which registers its own +// dependency. +MagicSelector( + controller: c, + selector: (C c) => c.count, + builder: (int n) => Builder( + builder: (BuildContext inner) => + WDiv(className: WindTheme.of(inner).surface), + ), +) +``` + +### Equality + +Plain `==`, deliberately. A selector that returns a freshly built `List` or `Map` never matches its own cache, because Dart gives collections identity equality, and the subtree then rebuilds on every notification exactly as it would have without the widget. + +Deep comparison was the alternative and is worse where it matters: walking a ten thousand element list on every keystroke costs more than the rebuild it prevents. Select a scalar, a record, or an object whose identity is stable across notifications. + +> [!NOTE] +> `MagicSelector` does not replace `MagicBuilder`. Use `MagicBuilder` when the source already is a `ValueListenable`, such as `MagicFormData.processingListenable`; use `MagicSelector` when the source is the controller itself. + ## MagicTitle diff --git a/lib/magic.dart b/lib/magic.dart index 8f7aa68..fae9efc 100644 --- a/lib/magic.dart +++ b/lib/magic.dart @@ -88,6 +88,7 @@ export 'src/encryption/encryption_service_provider.dart'; // UI export 'src/ui/magic_builder.dart'; +export 'src/ui/magic_selector.dart'; export 'src/ui/magic_feedback.dart'; export 'src/ui/magic_view_registry.dart'; export 'src/ui/magic_view.dart'; diff --git a/lib/src/ui/magic_selector.dart b/lib/src/ui/magic_selector.dart new file mode 100644 index 0000000..3f504c1 --- /dev/null +++ b/lib/src/ui/magic_selector.dart @@ -0,0 +1,207 @@ +import 'package:flutter/widgets.dart'; + +import '../http/magic_controller.dart'; + +/// Rebuilds one subtree when one part of a controller changes, and leaves it +/// alone the rest of the time. +/// +/// [MagicController.refreshUI] notifies every listener, and +/// `MagicStatefulViewState` answers by calling `setState` on the whole view. +/// That is the right default: a controller does not know which of its fields a +/// screen reads, and a view that rebuilds is always correct. It stops being +/// cheap on a screen where one field changes often and most of the screen does +/// not care. A search field is the worked example: every keystroke is a +/// notification, and a consumer measured one keystroke rebuilding 220 styled +/// containers, almost none of which could have looked different. +/// +/// ```dart +/// MagicSelector( +/// controller: controller, +/// selector: (GuideController c) => c.countLabel, +/// builder: (String label) => WText(label), +/// ) +/// ``` +/// +/// ## How it avoids the rebuild +/// +/// It caches the widget the builder returned and, while the selected value +/// compares equal, returns that same INSTANCE. `Element.updateChild` short +/// circuits when the new widget is `==` to the mounted one, so an identical +/// instance ends the descent right there and the subtree is never visited. +/// That is what makes this work under a parent that rebuilds anyway: a widget +/// that merely skipped its own `setState` would still be rebuilt from above. +/// +/// ## The contract this buys +/// +/// [builder] must be a pure function of the value it is handed. A cached child +/// cannot see anything else the closure captured, so this is stale for as long +/// as `count` happens not to move: +/// +/// ```dart +/// // WRONG: `total` is captured, and nothing here watches it. +/// MagicSelector( +/// controller: c, +/// selector: (C c) => c.count, +/// builder: (int count) => WText('$count of $total'), +/// ) +/// ``` +/// +/// Select both instead. A Dart record has value equality, so it compares by +/// content and the cache still holds: +/// +/// ```dart +/// MagicSelector( +/// controller: c, +/// selector: (C c) => (c.count, c.total), +/// builder: ((int, int) v) => WText('${v.$1} of ${v.$2}'), +/// ) +/// ``` +/// +/// Reading an [InheritedWidget] INSIDE the cached subtree is fine and needs no +/// selection: `Theme.of`, `MediaQuery.of` and `WindTheme.of` register their own +/// dependency, and the framework rebuilds a dependent element directly rather +/// than through its parent. +/// +/// A lookup captured from the ENCLOSING build is the same hole as `total` +/// above, and a dark-mode toggle is a likelier way to meet it: +/// +/// ```dart +/// // WRONG: `context` is the view's, so a theme change rebuilds the view, the +/// // cache is served, and this subtree keeps the old theme. +/// builder: (int n) => WDiv(className: WindTheme.of(context).surface), +/// +/// // Right: the lookup runs inside the built subtree. +/// builder: (int n) => Builder( +/// builder: (BuildContext inner) => +/// WDiv(className: WindTheme.of(inner).surface), +/// ), +/// ``` +/// +/// ## Equality +/// +/// Plain `==`, deliberately. A selector that returns a freshly built `List` or +/// `Map` therefore never matches its own cache, because Dart gives collections +/// identity equality, and the subtree rebuilds every notification exactly as it +/// would have without this widget. Deep comparison was the alternative and is +/// worse where it matters: walking a ten thousand channel list on every +/// keystroke costs more than the rebuild it prevents. Select a scalar, a +/// record, or an object whose identity is stable across notifications. +/// +/// See also: +/// +/// * [MagicBuilder], for a plain [ValueListenable] with no selection step. +class MagicSelector extends StatefulWidget { + /// The controller to watch. + final C controller; + + /// Reads the one piece of [controller] this subtree depends on. + /// + /// Called on every notification, so keep it cheap: a field read or a + /// memoized getter, never a scan that the controller has not already cached. + final T Function(C controller) selector; + + /// Builds the subtree from the selected value, and from nothing else. + /// + /// Takes no [BuildContext] for the same reason [MagicBuilder] does not: the + /// value is the whole input. Wrap the result in a [Builder] if a descendant + /// needs a context of its own. + final Widget Function(T value) builder; + + /// Creates a [MagicSelector]. + const MagicSelector({ + super.key, + required this.controller, + required this.selector, + required this.builder, + }); + + @override + State> createState() => _MagicSelectorState(); +} + +class _MagicSelectorState + extends State> { + late T _value; + + /// The widget [MagicSelector.builder] last returned. + /// + /// Returning this instance again is the entire mechanism; see the class doc. + Widget? _child; + + @override + void initState() { + super.initState(); + _value = widget.selector(widget.controller); + widget.controller.addListener(_onNotified); + } + + @override + void didUpdateWidget(covariant MagicSelector oldWidget) { + super.didUpdateWidget(oldWidget); + + if (!identical(oldWidget.controller, widget.controller)) { + oldWidget.controller.removeListener(_onNotified); + widget.controller.addListener(_onNotified); + _value = widget.selector(widget.controller); + _child = null; + } + } + + // A changed `selector` or `builder` deliberately does NOT invalidate the + // cache, and that is the decision the whole widget rests on. Both are written + // inline in a parent's `build`, so both are a fresh closure on every parent + // rebuild and comparing them by identity would drop the cache every time, + // which is the case this exists to serve. A changed selector still takes + // effect the moment it returns a different value, because `build` re-reads + // it. A changed builder that would render differently from the same value is + // the one thing this cannot see, which is why the class doc makes purity a + // contract rather than a suggestion. + + @override + void reassemble() { + super.reassemble(); + + // Hot reload marks descendants dirty, so an edit INSIDE the cached subtree + // shows up on its own. An edit to the builder does not: the cached instance + // is what those descendants rebuild against, so changing + // `builder: (n) => Text('$n items')` to `Text('$n rows')` kept showing + // `items` until the selected value happened to move. Dropping the cache is + // free here, because reassemble only runs in debug. + _child = null; + } + + @override + void dispose() { + // `removeListener` during a notification is safe: `ChangeNotifier` + // tombstones the slot and compacts the list once the outer call finishes. + widget.controller.removeListener(_onNotified); + super.dispose(); + } + + void _onNotified() { + if (!mounted) return; + + final T next = widget.selector(widget.controller); + if (next == _value) return; + + setState(() { + _value = next; + _child = null; + }); + } + + @override + Widget build(BuildContext context) { + // Re-read here as well as in the listener. A parent can rebuild this widget + // without any notification having fired (a `setState` higher up), and the + // cached child would then outlive the value it was built from. That covers + // a stale VALUE only; a hot-reloaded BUILDER is `reassemble`'s job. + final T next = widget.selector(widget.controller); + if (next != _value) { + _value = next; + _child = null; + } + + return _child ??= widget.builder(_value); + } +} diff --git a/skills/magic-framework/SKILL.md b/skills/magic-framework/SKILL.md index 1b6f713..da6eaf7 100644 --- a/skills/magic-framework/SKILL.md +++ b/skills/magic-framework/SKILL.md @@ -2,10 +2,10 @@ name: magic-framework description: "Write correct, idiomatic code in a Flutter app that depends on the `magic` framework (Laravel-inspired: IoC container, 18 facades, Eloquent-style ORM, service providers, reactive controllers, GoRouter routing, validation, auth, broadcasting). Use whenever code imports `package:magic/magic.dart` or `package:magic/testing.dart`, or the work touches Magic.init, MagicApp, a facade (Auth/Http/Cache/DB/Echo/Event/Gate/Config/Lang/Launch/Log/Pick/MagicRoute/Schema/Session/Storage/Vault/Crypt), a Model, MagicController, a MagicView, MagicFormData, FormRequest, a ServiceProvider, a migration, or the artisan make:* CLI. UI styling is Wind (separate wind-ui skill). Do NOT use for plain Flutter or Wind-only work with no magic import." when_to_use: "Use proactively when editing or scaffolding a magic app: Magic.init / a facade / a Model / a MagicController or MagicView / a form (MagicFormData, FormRequest, Validator) / a ServiceProvider / a route or MagicMiddleware / a migration / MagicStateMixin + RxStatus + fetchList / Session flash + old() + trans() / testing with MagicTest + Http.fake/Auth.fake / the artisan make:* CLI / the magic_deeplink, magic_notifications, magic_social_auth, magic_starter, magic_payments, or magic_devtools plugins. Trigger even when the user does not say the word 'magic'. Do NOT trigger for plain Flutter or Wind-only UI with no package:magic import." -version: 0.1.12 +version: 0.1.13 --- - + # Magic Framework @@ -30,7 +30,7 @@ Hard constraints for every line of magic code. 3. **Controllers are singletons.** `static X get instance => Magic.findOrPut(X.new);` is the canonical accessor. Views resolve controllers via `Magic.find()` (automatic in `MagicView`), never through constructors. 4. **IoC over `new` for services.** Bind in a provider's `register()`, resolve via the facade or `Magic.make('key')`. Do not scatter `Service()` construction across the app. 5. **Provider discipline.** `register()` is synchronous and is where routes and bindings go. `boot()` is async and may resolve other services; set `Auth.manager.setUserFactory(...)` here. -6. **Reactive state, not setState.** Controllers extend `MagicController` (a `ChangeNotifier`); state flows through `MagicStateMixin` + `RxStatus`. Use `refreshUI()` (guarded `notifyListeners`, and the single seam every controller notification goes through, including validation), `setLoading/setSuccess/setError/setEmpty`, and `MagicBuilder` for sections. `MagicController.onRefreshUI` is a null-by-default static debug tooling sets to observe those notifications. Local `setState` belongs only to genuine widget-local UI state inside a `MagicStatefulView`. +6. **Reactive state, not setState.** Controllers extend `MagicController` (a `ChangeNotifier`); state flows through `MagicStateMixin` + `RxStatus`. Use `refreshUI()` (guarded `notifyListeners`, and the single seam every controller notification goes through, including validation), `setLoading/setSuccess/setError/setEmpty`, `MagicBuilder` for a section backed by a `ValueListenable`, and `MagicSelector` for a section backed by a plain controller field (it caches its child, so it survives the parent's `setState` and is the tool for a field that changes on every keystroke). `MagicController.onRefreshUI` is a null-by-default static debug tooling sets to observe those notifications. Local `setState` belongs only to genuine widget-local UI state inside a `MagicStatefulView`. 7. **Typed attribute access.** Models use `get('key')` and `set('key', v)`, never raw `getAttribute`. Declare `fillable`; use `fill(validated, strict: true)` after validation so schema drift throws `MassAssignmentException`. 8. **Context-free navigation and feedback.** `MagicRoute.to/back/replace`, `Magic.snackbar/toast/dialog/confirm/loading`. Never depend on a `BuildContext` for navigation or feedback. Never navigate or fetch inside `build()`. 9. **Validate at the boundary.** `MagicFormData` for forms, `FormRequest` for complex payloads, `Validator` for ad hoc checks. Surface server errors with `handleApiError(response)` (from the `ValidatesRequests` mixin). @@ -382,7 +382,7 @@ Every path below is relative to this skill's own directory, `${CLAUDE_SKILL_DIR} | `references/bootstrap-lifecycle.md` | app bootstrap, IoC API, ServiceProvider, Env/Config, the Laravel mapping + divergences | | `references/facades-api.md` | any facade method signature or return type | | `references/eloquent-orm.md` | models, casts, relations, mass assignment, hybrid persistence, query builder, migrations | -| `references/controllers-views.md` | controllers, `MagicStateMixin`, `RxStatus`, views, `MagicBuilder`, `MagicCan` | +| `references/controllers-views.md` | controllers, `MagicStateMixin`, `RxStatus`, views, `MagicBuilder`, `MagicSelector`, `MagicCan` | | `references/forms-validation.md` | `MagicFormData`, `FormRequest`, `ValidatesRequests`, rules, async validation, `Session` flash | | `references/routing-navigation.md` | routes, `resource()`, middleware, params, URL strategy, page titles, `Session.tick` wiring | | `references/http-network.md` | `Http`, `MagicResponse`, `MagicNetworkInterceptor`, `configureDriver`, network config, `MagicPaginator` (url + fetcher) + `MagicPage` + `MagicPaginatedListView` | diff --git a/skills/magic-framework/references/controllers-views.md b/skills/magic-framework/references/controllers-views.md index 1f7b6e5..c7900cf 100644 --- a/skills/magic-framework/references/controllers-views.md +++ b/skills/magic-framework/references/controllers-views.md @@ -14,6 +14,7 @@ Laravel-inspired UI architecture. Controllers manage state and business logic; V - [MagicStatefulView\](#magicstatefulviewt) - [MagicResponsiveView\](#magicresponsiveviewt) - [MagicBuilder\](#magicbuildert) +- [MagicSelector\](#magicselectorc-t) - [Complete Lifecycle Example](#complete-lifecycle-example) - [Gotchas](#gotchas) @@ -399,6 +400,39 @@ MagicBuilder( Use `ValueListenableBuilder` directly when you need `BuildContext` or the `child` optimisation inside the builder. +## MagicSelector\ + +Rebuilds one subtree when one part of a controller changes. Use it when the thing to watch is a plain field on a `MagicController` rather than a `ValueListenable`, which is where `MagicBuilder` cannot help. + +```dart +class MagicSelector extends StatefulWidget { + const MagicSelector({ + super.key, + required C controller, + required T Function(C controller) selector, + required Widget Function(T value) builder, + }); +} +``` + +```dart +MagicSelector( + controller: controller, + selector: (c) => c.countLabel, + builder: (label) => WText(label), +) +``` + +It caches the widget the builder returned and, while the selected value compares equal, returns that same INSTANCE. `Element.updateChild` short circuits on `child.widget == newWidget`, so the subtree is never visited. That is what makes it survive a parent that rebuilds anyway, which is every `MagicStatefulView` under `refreshUI()`. + +Two rules follow from the caching: + +- `builder` must be a pure function of the selected value. A captured variable that changes without the value changing goes stale. Select a record to watch several fields: `selector: (c) => (c.count, c.total)`. +- Equality is plain `==`. A selector returning a freshly built `List` or `Map` never matches its own cache (Dart gives collections identity equality) and rebuilds every notification. Deep comparison is deliberately not used: walking a ten thousand element list per keystroke costs more than the rebuild it prevents. + +Reading an `InheritedWidget` inside the cached subtree is fine and needs no selection; dependent elements are rebuilt directly rather than through their parent. + + ## Complete Lifecycle Example End-to-end: controller registration, data loading, view rendering, form submission. diff --git a/test/ui/magic_selector_test.dart b/test/ui/magic_selector_test.dart new file mode 100644 index 0000000..d54509e --- /dev/null +++ b/test/ui/magic_selector_test.dart @@ -0,0 +1,427 @@ +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +// `hide TextDirection`: the magic barrel blanket-exports `package:intl`, whose +// `TextDirection` CLASS shadows the `dart:ui` enum, so `_wrap` below would not +// compile. Tracked separately; it is not this change. +import 'package:magic/magic.dart' hide TextDirection; + +import 'widget_build_counter.dart'; + +/// Gives the `Text` widgets below a direction to lay out in. +Widget _wrap(Widget child) => + Directionality(textDirection: TextDirection.ltr, child: child); + +/// Controller with three independent fields, so a selector can be shown to +/// ignore the two it did not ask for. +final class ProfileController extends SimpleMagicController { + String name = 'ada'; + String query = ''; + int visits = 0; + + void rename(String value) { + name = value; + refreshUI(); + } + + void search(String value) { + query = value; + refreshUI(); + } + + void visit() { + visits++; + refreshUI(); + } + + /// How many listeners are attached right now. + /// + /// Counted here rather than read off `ChangeNotifier.hasListeners`, which is + /// `@protected` and only legal inside a subclass instance member. A test + /// asserting that a widget detached cleanly has to see the count from + /// outside, so the controller under test keeps its own. + int listeners = 0; + + @override + void addListener(VoidCallback listener) { + listeners++; + super.addListener(listener); + } + + @override + void removeListener(VoidCallback listener) { + listeners--; + super.removeListener(listener); + } +} + +/// A view that puts one probe inside a selector and one outside it. +/// +/// The one outside is the control: it stands for everything on a real screen +/// that a keystroke cannot change, and its count is what a full-view rebuild +/// inflates. +final class ProfileView extends MagicStatefulView { + final ValueNotifier scopedBuilds; + final ValueNotifier siblingBuilds; + + const ProfileView({ + super.key, + required this.scopedBuilds, + required this.siblingBuilds, + }); + + @override + State createState() => _ProfileViewState(); +} + +class _ProfileViewState + extends MagicStatefulViewState { + @override + Widget build(BuildContext context) { + return Column( + children: [ + WidgetBuildCounter( + counter: widget.siblingBuilds, + child: const Text('static'), + ), + MagicSelector( + controller: controller, + selector: (ProfileController c) => c.name, + builder: (String name) => WidgetBuildCounter( + counter: widget.scopedBuilds, + child: Text(name), + ), + ), + ], + ); + } +} + +void main() { + setUp(() { + MagicApp.reset(); + Magic.flush(); + }); + + group('under a rebuilding parent', () { + late ProfileController controller; + late ValueNotifier scoped; + late ValueNotifier sibling; + + Future pump(WidgetTester tester) async { + controller = ProfileController(); + Magic.put(controller); + scoped = ValueNotifier(0); + sibling = ValueNotifier(0); + + await tester.pumpWidget( + _wrap(ProfileView(scopedBuilds: scoped, siblingBuilds: sibling)), + ); + } + + testWidgets('an unrelated change leaves the scoped subtree alone', ( + tester, + ) async { + await pump(tester); + expect(scoped.value, 1); + expect(sibling.value, 1); + + controller.search('bbc'); + await tester.pump(); + + // The whole view rebuilt, which is what `refreshUI` means today, and the + // sibling proves it. The selector still returned the same name, so its + // subtree was never asked to build. + expect(sibling.value, 2); + expect(scoped.value, 1); + }); + + testWidgets('many unrelated changes still cost the subtree nothing', ( + tester, + ) async { + await pump(tester); + + for (int i = 0; i < 7; i++) { + controller.search('term $i'); + await tester.pump(); + } + + expect(sibling.value, 8); + expect(scoped.value, 1); + }); + + testWidgets('a change to the selected value rebuilds the subtree', ( + tester, + ) async { + await pump(tester); + + controller.rename('grace'); + await tester.pump(); + + expect(scoped.value, 2); + expect(find.text('grace'), findsOneWidget); + }); + + testWidgets('and the new value reaches the builder, not a stale one', ( + tester, + ) async { + await pump(tester); + + controller.rename('grace'); + await tester.pump(); + controller.rename('hopper'); + await tester.pump(); + + expect(scoped.value, 3); + expect(find.text('hopper'), findsOneWidget); + }); + }); + + group('standing alone', () { + testWidgets('it listens to the controller without a MagicStatefulView', ( + tester, + ) async { + final ProfileController controller = ProfileController(); + final ValueNotifier builds = ValueNotifier(0); + + await tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: (ProfileController c) => c.visits, + builder: (int visits) => + WidgetBuildCounter(counter: builds, child: Text('$visits')), + ), + ), + ); + + expect(find.text('0'), findsOneWidget); + + controller.visit(); + await tester.pump(); + + expect(find.text('1'), findsOneWidget); + expect(builds.value, 2); + }); + + testWidgets('a notification that does not move the value builds nothing', ( + tester, + ) async { + final ProfileController controller = ProfileController(); + final ValueNotifier builds = ValueNotifier(0); + + await tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: (ProfileController c) => c.name, + builder: (String name) => + WidgetBuildCounter(counter: builds, child: Text(name)), + ), + ), + ); + + controller.visit(); + controller.visit(); + await tester.pump(); + + expect(builds.value, 1); + }); + + testWidgets('it swaps listeners when the controller instance changes', ( + tester, + ) async { + final ProfileController first = ProfileController(); + final ProfileController second = ProfileController()..name = 'grace'; + + Future pumpWith(ProfileController c) { + return tester.pumpWidget( + _wrap( + MagicSelector( + controller: c, + selector: (ProfileController x) => x.name, + builder: (String name) => Text(name), + ), + ), + ); + } + + await pumpWith(first); + expect(find.text('ada'), findsOneWidget); + + await pumpWith(second); + expect(find.text('grace'), findsOneWidget); + + // The old controller must no longer drive this widget, or a replaced + // controller keeps a live listener and the screen answers to two sources. + expect(first.listeners, 0); + expect(second.listeners, 1); + }); + + testWidgets('a changed selector re-reads rather than serving the cache', ( + tester, + ) async { + final ProfileController controller = ProfileController(); + + Future pumpWith(String Function(ProfileController) selector) { + return tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: selector, + builder: (String value) => Text(value), + ), + ), + ); + } + + await pumpWith((ProfileController c) => c.name); + expect(find.text('ada'), findsOneWidget); + + controller.search('bbc'); + await pumpWith((ProfileController c) => c.query); + + expect(find.text('bbc'), findsOneWidget); + }); + + testWidgets('it stops listening when removed from the tree', ( + tester, + ) async { + final ProfileController controller = ProfileController(); + + await tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: (ProfileController c) => c.name, + builder: (String name) => Text(name), + ), + ), + ); + expect(controller.listeners, 1); + + await tester.pumpWidget(_wrap(const SizedBox.shrink())); + + expect(controller.listeners, 0); + }); + }); + + group('the caching contract, pinned rather than fixed', () { + testWidgets('a hot reload drops the cache', (tester) async { + // Without this, an edit to the builder is invisible until the selected + // value happens to move: hot reload marks descendants dirty, but the + // cached instance is what they rebuild against. + final ProfileController controller = ProfileController(); + final ValueNotifier builds = ValueNotifier(0); + + await tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: (ProfileController c) => c.name, + builder: (String name) => + WidgetBuildCounter(counter: builds, child: Text(name)), + ), + ), + ); + expect(builds.value, 1); + + // What `flutter run`'s `r` triggers. + tester.binding.reassembleApplication(); + await tester.pump(); + + expect(builds.value, 2); + }); + + testWidgets('a changed builder is NOT seen while the value holds', ( + tester, + ) async { + // The hole the purity contract exists to rule out, written down so the + // next reader meets it as a decision rather than as a surprise. Nothing + // here is a fix: it documents what the cache costs. + final ProfileController controller = ProfileController(); + + Future pumpWith(String suffix) { + return tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: (ProfileController c) => c.name, + builder: (String name) => Text('$name $suffix'), + ), + ), + ); + } + + await pumpWith('one'); + expect(find.text('ada one'), findsOneWidget); + + await pumpWith('two'); + + expect(find.text('ada one'), findsOneWidget, reason: 'still cached'); + expect(find.text('ada two'), findsNothing); + + // And it takes effect the moment the value moves. + controller.rename('grace'); + await tester.pump(); + expect(find.text('grace two'), findsOneWidget); + }); + }); + + group('the equality contract', () { + testWidgets('a record selects several fields at once', (tester) async { + // The documented way to watch more than one field. A record has value + // equality, so it compares by content and the cache holds. + final ProfileController controller = ProfileController(); + final ValueNotifier builds = ValueNotifier(0); + + await tester.pumpWidget( + _wrap( + MagicSelector( + controller: controller, + selector: (ProfileController c) => (c.name, c.visits), + builder: ((String, int) value) => WidgetBuildCounter( + counter: builds, + child: Text('${value.$1}/${value.$2}'), + ), + ), + ), + ); + + controller.search('irrelevant'); + await tester.pump(); + expect(builds.value, 1); + + controller.visit(); + await tester.pump(); + expect(builds.value, 2); + expect(find.text('ada/1'), findsOneWidget); + }); + + testWidgets('a freshly built list rebuilds every time, as documented', ( + tester, + ) async { + // Dart's `List` has identity equality, so a selector that builds one is a + // selector that never matches its own cache. This is pinned rather than + // fixed: a deep comparison of a ten thousand element list on every + // notification costs more than the rebuild it would prevent. + final ProfileController controller = ProfileController(); + final ValueNotifier builds = ValueNotifier(0); + + await tester.pumpWidget( + _wrap( + MagicSelector>( + controller: controller, + selector: (ProfileController c) => [c.name], + builder: (List value) => + WidgetBuildCounter(counter: builds, child: Text(value.first)), + ), + ), + ); + + controller.visit(); + await tester.pump(); + + expect(builds.value, 2); + }); + }); +}