diff --git a/packages/view_model/CHANGELOG.md b/packages/view_model/CHANGELOG.md index 89785e8..d85b90e 100644 --- a/packages/view_model/CHANGELOG.md +++ b/packages/view_model/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.0.2 +- Fix listener mutation safety during notification dispatch. +- Fix recreate failure handling to keep previous instance valid. +- Fix disposal cleanup in instance manager and pause providers. +- Improve cached access error handling and DevTools active state reporting. +- Internal cleanup: remove duplicated binding extension implementations. + ## 1.0.1 - Fix: DevToolsService not clearing singleton `_instance` on dispose - Fix: typo "suucess" → "success" in DevToolsService log diff --git a/packages/view_model/analysis_options.yaml b/packages/view_model/analysis_options.yaml index 4ffd5b6..6cc57dc 100644 --- a/packages/view_model/analysis_options.yaml +++ b/packages/view_model/analysis_options.yaml @@ -1,6 +1,10 @@ # Additional information about this file can be found at # https://dart.dev/guides/language/analysis-options +analyzer: + exclude: + - example/** + linter: # The lint rules applied to this project can be customized in the # section below to disable rules from the `package:flutter_lints/flutter.yaml` diff --git a/packages/view_model/lib/src/devtool/service.dart b/packages/view_model/lib/src/devtool/service.dart index 000b439..d4389a1 100644 --- a/packages/view_model/lib/src/devtool/service.dart +++ b/packages/view_model/lib/src/devtool/service.dart @@ -205,7 +205,7 @@ class DevToolsService { 'id': vm.instanceId, 'type': vm.typeName, 'label': '${vm.typeName}\n${vm.key ?? vm.instanceId}', - 'isActive': true, + 'isActive': !vm.isDisposed, }) .toList(), 'edges': dependencies, diff --git a/packages/view_model/lib/src/get_instance/auto_dispose.dart b/packages/view_model/lib/src/get_instance/auto_dispose.dart index 66e7fec..293b969 100644 --- a/packages/view_model/lib/src/get_instance/auto_dispose.dart +++ b/packages/view_model/lib/src/get_instance/auto_dispose.dart @@ -162,7 +162,7 @@ class AutoDisposeInstanceController { /// the given action to each ViewModel instance. void performForAllInstances(void Function(ViewModel viewModel) action) { for (final notifier in _instanceNotifiers) { - if (notifier.instance is ViewModel) { + if (!notifier.isDisposed && notifier.instance is ViewModel) { action(notifier.instance as ViewModel); } } @@ -184,7 +184,11 @@ class AutoDisposeInstanceController { /// ``` void recycle(Object instance) { _instanceNotifiers.removeWhere((e) { - if (e.instance == instance) { + if (!e.isDisposed && e.instance == instance) { + final listener = _notifierListeners.remove(e); + if (listener != null) { + e.removeListener(listener); + } e.unbindAll(); return true; } else { @@ -199,7 +203,7 @@ class AutoDisposeInstanceController { /// binders remain, the instance can be recycled automatically. void unbindInstance(Object instance) { for (final e in _instanceNotifiers) { - if (e.instance == instance) { + if (!e.isDisposed && e.instance == instance) { e.unbind(viewModelBinding.id); break; } @@ -227,10 +231,12 @@ class AutoDisposeInstanceController { if (!e.isDisposed && e.instance is ViewModel) { (e.instance as ViewModel).refHandler.removeRef(viewModelBinding); } - e.unbind(viewModelBinding.id); + // Remove listener before unbind, because unbind may trigger _recycle() + // which disposes the ChangeNotifier, making removeListener fail. if (_notifierListeners.containsKey(e)) { e.removeListener(_notifierListeners[e]!); } + e.unbind(viewModelBinding.id); } _notifierListeners.clear(); _instanceNotifiers.clear(); diff --git a/packages/view_model/lib/src/get_instance/manager.dart b/packages/view_model/lib/src/get_instance/manager.dart index 8d97e1c..c8f3288 100644 --- a/packages/view_model/lib/src/get_instance/manager.dart +++ b/packages/view_model/lib/src/get_instance/manager.dart @@ -8,6 +8,7 @@ /// @author luwenjie on 2025/3/25 12:23:33 library; +import 'package:flutter/foundation.dart'; import 'package:view_model/src/view_model/state_store.dart'; import 'store.dart'; @@ -52,7 +53,11 @@ class InstanceManager { T t, { T Function()? builder, }) { - final Store s = _stores[T]; + final store = _stores[T]; + if (store is! Store) { + throw ViewModelError("Cannot recreate $T instance. Store not found."); + } + final Store s = store; return s.recreate( t, builder: builder, @@ -78,10 +83,22 @@ class InstanceManager { /// /// Returns the [Store] instance for type [T]. Store _getStore() { - Store? s = _stores[T]; - s ??= Store(); - _stores[T] = s; - return s; + final cached = _stores[T]; + if (cached is Store) { + return cached; + } + + late final Store created; + created = Store( + onStoreEmpty: () { + final current = _stores[T]; + if (!identical(current, created) || !created.isEmpty) return; + _stores.remove(T); + created.dispose(); + }, + ); + _stores[T] = created; + return created; } /// Gets a ViewModel instance directly. @@ -175,6 +192,12 @@ class InstanceManager { } return _getStore().getInstancesByTag(tag); } + + @visibleForTesting + int get debugStoreCount => _stores.length; + + @visibleForTesting + bool debugHasStoreFor() => _stores[T] != null; } /// Factory configuration for ViewModel instance creation and retrieval. diff --git a/packages/view_model/lib/src/get_instance/store.dart b/packages/view_model/lib/src/get_instance/store.dart index e05c3a1..4448b9b 100644 --- a/packages/view_model/lib/src/get_instance/store.dart +++ b/packages/view_model/lib/src/get_instance/store.dart @@ -36,11 +36,18 @@ class _Undefined { /// The store maintains a map of instances keyed by their unique identifiers /// and automatically handles cleanup when instances are no longer needed. class Store { + Store({void Function()? onStoreEmpty}) : _onStoreEmpty = onStoreEmpty; + + final void Function()? _onStoreEmpty; + /// Stream controller for broadcasting instance creation events. final _streamController = StreamController>.broadcast(); /// Map of cached instances keyed by their unique identifiers. final Map> _instances = {}; + bool _disposed = false; + + bool get isEmpty => _instances.isEmpty; /// Finds the most recently created instance, optionally filtered by tag. /// @@ -106,6 +113,9 @@ class Store { case InstanceAction.dispose: _instances.remove(notifier.arg.key); notifier.removeListener(onNotify); + if (_instances.isEmpty) { + _onStoreEmpty?.call(); + } break; case InstanceAction.recreate: break; @@ -136,6 +146,9 @@ class Store { /// Throws [ViewModelError] if no cached instance exists and no /// builder is provided. InstanceHandle getNotifier({required InstanceFactory factory}) { + if (_disposed) { + throw ViewModelError("Store<$T> has been disposed."); + } final realKey = factory.arg.key ?? Object(); final bindingId = factory.arg.bindingId; final arg = factory.arg.copyWith( @@ -199,6 +212,13 @@ class Store { ); return find.recreate(builder: builder); } + + void dispose() { + if (_disposed) return; + _disposed = true; + _instances.clear(); + _streamController.close(); + } } /// Handle for managing a ViewModel instance and its lifecycle. @@ -235,6 +255,7 @@ class InstanceHandle with ChangeNotifier { /// The actual ViewModel instance (null when disposed). late T? _instance; + bool _disposed = false; bool get isDisposed => _instance == null; @@ -334,8 +355,14 @@ class InstanceHandle with ChangeNotifier { T recreate({ T Function()? builder, }) { - onDispose(); - _instance = (builder?.call()) ?? factory.call(); + final previous = _instance; + if (previous == null) { + throw ViewModelError( + "Cannot recreate $T instance. Instance is disposed."); + } + final recreated = (builder?.call()) ?? factory.call(); + _tryCallInstanceDispose(previous); + _instance = recreated; onCreate(arg); _action = InstanceAction.recreate; notifyListeners(); @@ -367,12 +394,12 @@ class InstanceHandle with ChangeNotifier { /// This method attempts to call the onDispose lifecycle method if the /// instance implements [InstanceLifeCycle]. Errors are logged but don't /// prevent the disposal process. - void _tryCallInstanceDispose() { - if (_instance is InstanceLifeCycle) { + void _tryCallInstanceDispose(Object? target) { + if (target is InstanceLifeCycle) { try { - (_instance as InstanceLifeCycle).onDispose(arg); + target.onDispose(arg); } catch (e) { - viewModelLog("${_instance.runtimeType} onDispose error $e"); + viewModelLog("${target.runtimeType} onDispose error $e"); } } } @@ -382,8 +409,11 @@ class InstanceHandle with ChangeNotifier { /// This method calls the instance's disposal lifecycle method, /// clears all watchers, and nullifies the instance reference. void onDispose() { - _tryCallInstanceDispose(); + if (_disposed) return; + _disposed = true; + _tryCallInstanceDispose(_instance); _instance = null; + super.dispose(); } } diff --git a/packages/view_model/lib/src/view_model/pause_provider.dart b/packages/view_model/lib/src/view_model/pause_provider.dart index bf8bbc9..4d0b144 100644 --- a/packages/view_model/lib/src/view_model/pause_provider.dart +++ b/packages/view_model/lib/src/view_model/pause_provider.dart @@ -45,10 +45,9 @@ class AppPauseProvider with ViewModelBindingPauseProvider { @override void dispose() { - super.dispose(); _subscription?.cancel(); _subscription = null; - _controller.close(); + super.dispose(); } } @@ -80,9 +79,9 @@ class TickerModePauseProvider with ViewModelBindingPauseProvider { @override void dispose() { - super.dispose(); _notifier?.removeListener(_onChange); _notifier = null; + super.dispose(); } } @@ -120,8 +119,8 @@ class PageRoutePauseProvider with ViewModelBindingPauseProvider, RouteAware { @override void dispose() { - super.dispose(); unsubscribe(_observer); + super.dispose(); } @override diff --git a/packages/view_model/lib/src/view_model/view_model.dart b/packages/view_model/lib/src/view_model/view_model.dart index fb3b5e1..59f6380 100644 --- a/packages/view_model/lib/src/view_model/view_model.dart +++ b/packages/view_model/lib/src/view_model/view_model.dart @@ -79,7 +79,8 @@ class ChangeNotifierViewModel extends ChangeNotifier with ViewModel {} /// } /// } /// ``` -mixin class ViewModel implements InstanceLifeCycle, Listenable { +mixin class ViewModel + implements InstanceLifeCycle, Listenable, ViewModelBindingHost { /// Returns the [ViewModelBinding] interface for accessing other ViewModels. /// /// This property allows you to use @@ -135,7 +136,7 @@ mixin class ViewModel implements InstanceLifeCycle, Listenable { static T? maybeReadCached({Object? key, Object? tag}) { try { return readCached(key: key, tag: tag); - } catch (e) { + } on ViewModelError { return null; } } @@ -376,7 +377,9 @@ mixin class ViewModel implements InstanceLifeCycle, Listenable { /// is provided, errors are logged. This prevents one listener from /// affecting others. void notifyListeners() { - for (final element in _listeners) { + final listeners = List.of(_listeners); + for (final element in listeners) { + if (!_listeners.contains(element)) continue; try { element.call(); } catch (e, stack) { @@ -634,7 +637,10 @@ abstract class StateViewModel with ViewModel { if (_isDisposed) return; // Phase 1: Notify state listeners with previous and current state - for (final element in _stateListeners) { + final stateListeners = + List.of(_stateListeners); + for (final element in stateListeners) { + if (!_stateListeners.contains(element)) continue; try { element.call(event.previousState, event.currentState); } catch (e, stack) { @@ -648,7 +654,9 @@ abstract class StateViewModel with ViewModel { } // Phase 2: Notify general listeners - for (final element in _listeners) { + final listeners = List.of(_listeners); + for (final element in listeners) { + if (!_listeners.contains(element)) continue; try { element.call(); } catch (e, stack) { diff --git a/packages/view_model/lib/src/view_model/view_model_binding.dart b/packages/view_model/lib/src/view_model/view_model_binding.dart index f76ec17..f01cfc1 100644 --- a/packages/view_model/lib/src/view_model/view_model_binding.dart +++ b/packages/view_model/lib/src/view_model/view_model_binding.dart @@ -12,9 +12,6 @@ import 'package:view_model/src/view_model/pause_provider.dart'; import 'package:view_model/src/view_model/util.dart'; import 'package:view_model/src/view_model/binding_zone.dart'; import 'package:view_model/src/view_model/view_model.dart'; -import 'package:view_model/src/view_model/widget_mixin/stateful_extension.dart'; -// ignore: lines_longer_than_80_chars -import 'package:view_model/src/view_model/widget_mixin/stateless_extension.dart'; import 'state_store.dart'; @@ -98,6 +95,14 @@ abstract interface class ViewModelBindingInterface { void recycle(VM viewModel); } +/// Common host interface for types exposing a [viewModelBinding] accessor. +/// +/// This enables shared extension methods across different host types, such as +/// [ViewModel], [ViewModelStateMixin], and [ViewModelStatelessMixin]. +abstract interface class ViewModelBindingHost { + ViewModelBindingInterface get viewModelBinding; +} + /// Core abstraction for managing ViewModel lifecycle and dependency /// injection. /// @@ -499,7 +504,8 @@ mixin class ViewModelBinding implements ViewModelBindingInterface { getName(); } if (isDisposed) { - throw ViewModelError("$VM was Disposed!"); + throw ViewModelError( + "Cannot get $VM: ViewModelBinding(${getName()}) is already disposed."); } if (VM == ViewModel || VM == dynamic) { throw ViewModelError("VM must extends ViewModel"); @@ -641,7 +647,7 @@ mixin class ViewModelBinding implements ViewModelBindingInterface { key: key, tag: tag, ); - } catch (e) { + } on ViewModelError { return null; } } @@ -711,201 +717,16 @@ mixin class ViewModelBinding implements ViewModelBindingInterface { VM? maybeReadCached({Object? key, Object? tag}) { try { return readCached(key: key, tag: tag); - } catch (e) { + } on ViewModelError { // return null; } } } -extension StateViewModelBindingExtension on ViewModelStateMixin { - VM watchViewModel( - {required ViewModelFactory factory}) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.watch(factory); - } - - void listenViewModel({ - required ViewModelFactory factory, - required VoidCallback onChanged, - }) { - // ignore: invalid_use_of_protected_member - viewModelBinding.listen(factory, onChanged: onChanged); - } - - void listenViewModelState, S>({ - required ViewModelFactory factory, - required Function(S? previous, S state) onChanged, - }) { - // ignore: invalid_use_of_protected_member - viewModelBinding.listenState(factory, onChanged: onChanged); - } - - void listenViewModelStateSelect, S, R>({ - required ViewModelFactory factory, - required R Function(S state) selector, - required Function(R? previous, R current) onChanged, - }) { - // ignore: invalid_use_of_protected_member - viewModelBinding.listenStateSelect( - factory, - selector: selector, - onChanged: onChanged, - ); - } - - void recycleViewModel(VM viewModel) { - // ignore: invalid_use_of_protected_member - viewModelBinding.recycle(viewModel); - } - - VM readViewModel( - {required ViewModelFactory factory}) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.read(factory); - } - - VM readCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.readCached( - key: key, - tag: tag, - ); - } - - VM watchCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.watchCached( - key: key, - tag: tag, - ); - } - - VM? maybeWatchCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.maybeWatchCached( - key: key, - tag: tag, - ); - } - - VM? maybeReadCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.maybeReadCached( - key: key, - tag: tag, - ); - } -} - -extension StatelessWidgetViewModelBindingExtension on ViewModelStatelessMixin { - VM watchViewModel( - {required ViewModelFactory factory}) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.watch(factory); - } - - void listenViewModel({ - required ViewModelFactory factory, - required VoidCallback onChanged, - }) { - // ignore: invalid_use_of_protected_member - viewModelBinding.listen(factory, onChanged: onChanged); - } - - void listenViewModelState, S>({ - required ViewModelFactory factory, - required Function(S? previous, S state) onChanged, - }) { - // ignore: invalid_use_of_protected_member - viewModelBinding.listenState(factory, onChanged: onChanged); - } - - void listenViewModelStateSelect, S, R>({ - required ViewModelFactory factory, - required R Function(S state) selector, - required Function(R? previous, R current) onChanged, - }) { - // ignore: invalid_use_of_protected_member - viewModelBinding.listenStateSelect( - factory, - selector: selector, - onChanged: onChanged, - ); - } - - void recycleViewModel(VM viewModel) { - // ignore: invalid_use_of_protected_member - viewModelBinding.recycle(viewModel); - } - - VM readViewModel( - {required ViewModelFactory factory}) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.read(factory); - } - - VM readCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.readCached( - key: key, - tag: tag, - ); - } - - VM watchCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.watchCached( - key: key, - tag: tag, - ); - } - - VM? maybeWatchCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.maybeWatchCached( - key: key, - tag: tag, - ); - } - - VM? maybeReadCachedViewModel({ - Object? key, - Object? tag, - }) { - // ignore: invalid_use_of_protected_member - return viewModelBinding.maybeReadCached( - key: key, - tag: tag, - ); - } -} - -extension ViewModelBindingExtension on ViewModel { +extension ViewModelBindingHostExtension on ViewModelBindingHost { VM watchViewModel( {required ViewModelFactory factory}) { - // ignore: invalid_use_of_protected_member return viewModelBinding.watch(factory); } @@ -913,7 +734,6 @@ extension ViewModelBindingExtension on ViewModel { required ViewModelFactory factory, required VoidCallback onChanged, }) { - // ignore: invalid_use_of_protected_member viewModelBinding.listen(factory, onChanged: onChanged); } @@ -921,7 +741,6 @@ extension ViewModelBindingExtension on ViewModel { required ViewModelFactory factory, required Function(S? previous, S state) onChanged, }) { - // ignore: invalid_use_of_protected_member viewModelBinding.listenState(factory, onChanged: onChanged); } @@ -930,7 +749,6 @@ extension ViewModelBindingExtension on ViewModel { required R Function(S state) selector, required Function(R? previous, R current) onChanged, }) { - // ignore: invalid_use_of_protected_member viewModelBinding.listenStateSelect( factory, selector: selector, @@ -939,13 +757,11 @@ extension ViewModelBindingExtension on ViewModel { } void recycleViewModel(VM viewModel) { - // ignore: invalid_use_of_protected_member viewModelBinding.recycle(viewModel); } VM readViewModel( {required ViewModelFactory factory}) { - // ignore: invalid_use_of_protected_member return viewModelBinding.read(factory); } @@ -953,7 +769,6 @@ extension ViewModelBindingExtension on ViewModel { Object? key, Object? tag, }) { - // ignore: invalid_use_of_protected_member return viewModelBinding.readCached( key: key, tag: tag, @@ -964,7 +779,6 @@ extension ViewModelBindingExtension on ViewModel { Object? key, Object? tag, }) { - // ignore: invalid_use_of_protected_member return viewModelBinding.watchCached( key: key, tag: tag, @@ -975,7 +789,6 @@ extension ViewModelBindingExtension on ViewModel { Object? key, Object? tag, }) { - // ignore: invalid_use_of_protected_member return viewModelBinding.maybeWatchCached( key: key, tag: tag, @@ -986,7 +799,6 @@ extension ViewModelBindingExtension on ViewModel { Object? key, Object? tag, }) { - // ignore: invalid_use_of_protected_member return viewModelBinding.maybeReadCached( key: key, tag: tag, diff --git a/packages/view_model/lib/src/view_model/widget_mixin/stateful_extension.dart b/packages/view_model/lib/src/view_model/widget_mixin/stateful_extension.dart index 63a42a0..5663a5b 100644 --- a/packages/view_model/lib/src/view_model/widget_mixin/stateful_extension.dart +++ b/packages/view_model/lib/src/view_model/widget_mixin/stateful_extension.dart @@ -3,8 +3,8 @@ library; import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:view_model/src/view_model/pause_provider.dart'; +import 'package:view_model/src/view_model/view_model_binding.dart'; import 'package:view_model/src/view_model/widget_mixin/view_model_binding.dart'; -import 'package:view_model/view_model.dart'; /// Mixin that integrates ViewModels with Flutter's State lifecycle. /// @@ -39,7 +39,8 @@ import 'package:view_model/view_model.dart'; /// } /// } /// ``` -mixin ViewModelStateMixin on State { +mixin ViewModelStateMixin on State + implements ViewModelBindingHost { @protected late final WidgetViewModelBinding viewModelBinding = WidgetViewModelBinding( refreshWidget: _rebuildState, @@ -79,11 +80,11 @@ mixin ViewModelStateMixin on State { @override void dispose() { - super.dispose(); viewModelBinding.dispose(); _appPauseProvider.dispose(); _routePauseProvider.dispose(); _tickerModePauseProvider.dispose(); + super.dispose(); } void _rebuildState() { diff --git a/packages/view_model/lib/src/view_model/widget_mixin/stateless_extension.dart b/packages/view_model/lib/src/view_model/widget_mixin/stateless_extension.dart index 6962b34..7a73e83 100644 --- a/packages/view_model/lib/src/view_model/widget_mixin/stateless_extension.dart +++ b/packages/view_model/lib/src/view_model/widget_mixin/stateless_extension.dart @@ -2,6 +2,7 @@ import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; import 'package:view_model/src/view_model/pause_aware.dart'; import 'package:view_model/src/view_model/pause_provider.dart'; +import 'package:view_model/src/view_model/view_model_binding.dart'; import 'package:view_model/src/view_model/widget_mixin/view_model_binding.dart'; /// Stateless integration for ViewModel access from widgets. @@ -17,7 +18,8 @@ import 'package:view_model/src/view_model/widget_mixin/view_model_binding.dart'; /// > each widget instance creates exactly one Element. If the same widget /// > instance is mounted multiple times (e.g., via GlobalKey migration), /// > behavior may be unexpected. For complex use cases, use StatefulWidget. -mixin ViewModelStatelessMixin on StatelessWidget { +mixin ViewModelStatelessMixin on StatelessWidget + implements ViewModelBindingHost { /// The cached element for this widget. /// /// Using late final ensures consistent element-to-widget binding. diff --git a/packages/view_model/pubspec.lock b/packages/view_model/pubspec.lock index c25079f..aa8764f 100644 --- a/packages/view_model/pubspec.lock +++ b/packages/view_model/pubspec.lock @@ -428,10 +428,10 @@ packages: dependency: "direct main" description: name: view_model_annotation - sha256: "34bcf871134aa5808163539af656b6d1a288876f3ff6a58e9cf8d618451e7985" + sha256: "41ef3b28a35b5a87915d90d0b746aee59d25c54d0a2b6efbb0eec501e35e0dbe" url: "https://pub.dev" source: hosted - version: "1.0.1" + version: "1.0.2" vm_service: dependency: transitive description: diff --git a/packages/view_model/pubspec.yaml b/packages/view_model/pubspec.yaml index 3ab467f..abeaf51 100644 --- a/packages/view_model/pubspec.yaml +++ b/packages/view_model/pubspec.yaml @@ -2,7 +2,7 @@ name: view_model description: Everything is ViewModel. Enjoy automatic lifecycle management, prevent memory leaks, and share state effortlessly. Simple, lightweight, and powerful. -version: 1.0.1 +version: 1.0.2 homepage: https://github.com/lwj1994/flutter_view_model repository: https://github.com/lwj1994/flutter_view_model issue_tracker: https://github.com/lwj1994/flutter_view_model/issues @@ -30,7 +30,7 @@ dependencies: flutter: sdk: flutter meta: - view_model_annotation: 1.0.1 + view_model_annotation: 1.0.2 stack_trace: dev_dependencies: flutter_test: diff --git a/packages/view_model/test/auto_dispose_test.dart b/packages/view_model/test/auto_dispose_test.dart index 72c6521..fb9cc92 100644 --- a/packages/view_model/test/auto_dispose_test.dart +++ b/packages/view_model/test/auto_dispose_test.dart @@ -184,6 +184,28 @@ void main() { expect(vm2, isNot(equals(vm1))); }); + test('recreate keeps old instance when builder throws', () { + final factory = InstanceFactory( + builder: () => TestViewModel(state: 'initial'), + arg: const InstanceArg(key: 'recreate_builder_throw'), + ); + + final vm1 = controller.getInstance(factory: factory); + final handle = + instanceManager.getNotifier(factory: factory); + + expect( + () => instanceManager.recreate(vm1, builder: () { + throw StateError('recreate failure'); + }), + throwsA(isA()), + ); + + expect(identical(handle.instance, vm1), isTrue); + expect(vm1.isDisposed, isFalse); + expect(handle.action, isNull); + }); + test('recreate preserves watchers and bindings', () { // Test that recreate preserves watcher relationships final factory = InstanceFactory( diff --git a/packages/view_model/test/get_instance_test.dart b/packages/view_model/test/get_instance_test.dart index dd3ac23..31947ad 100644 --- a/packages/view_model/test/get_instance_test.dart +++ b/packages/view_model/test/get_instance_test.dart @@ -57,6 +57,10 @@ class ErrorDisposeModel implements InstanceLifeCycle { class UnusedModel {} +class AutoCleanupModel {} + +class AutoCleanupModelB {} + void main() { group('get_instance', () { setUp(() { @@ -417,5 +421,48 @@ void main() { expect(list[0].index > list[1].index, isTrue); expect(list[1].index > list[2].index, isTrue); }); + + test('removes store when last instance is disposed', () async { + final baseline = instanceManager.debugStoreCount; + final handle = instanceManager.getNotifier( + factory: InstanceFactory( + builder: () => AutoCleanupModel(), + arg: const InstanceArg(key: 'auto_cleanup_store_remove'), + ), + ); + + expect(instanceManager.debugHasStoreFor(), isTrue); + + handle.unbindAll(); + await Future.delayed(Duration.zero); + + expect(instanceManager.debugHasStoreFor(), isFalse); + expect(instanceManager.debugStoreCount, baseline); + }); + + test('keeps store while there are still alive instances', () async { + final handle1 = instanceManager.getNotifier( + factory: InstanceFactory( + builder: () => AutoCleanupModelB(), + arg: const InstanceArg(key: 'auto_cleanup_store_keep_1'), + ), + ); + final handle2 = instanceManager.getNotifier( + factory: InstanceFactory( + builder: () => AutoCleanupModelB(), + arg: const InstanceArg(key: 'auto_cleanup_store_keep_2'), + ), + ); + + expect(instanceManager.debugHasStoreFor(), isTrue); + + handle1.unbindAll(); + await Future.delayed(Duration.zero); + expect(instanceManager.debugHasStoreFor(), isTrue); + + handle2.unbindAll(); + await Future.delayed(Duration.zero); + expect(instanceManager.debugHasStoreFor(), isFalse); + }); }); } diff --git a/packages/view_model/test/view_model_binding_test.dart b/packages/view_model/test/view_model_binding_test.dart index d311ddd..708b0ea 100644 --- a/packages/view_model/test/view_model_binding_test.dart +++ b/packages/view_model/test/view_model_binding_test.dart @@ -14,6 +14,16 @@ class SimpleStateVM extends StateViewModel { SimpleStateVM({required SimpleState initial}) : super(state: initial); } +class ThrowingHashKey { + const ThrowingHashKey(); + + @override + int get hashCode => throw StateError('hashCode failure'); + + @override + bool operator ==(Object other) => identical(this, other); +} + class TestRef with ViewModelBinding {} class TestRefWithCounter with ViewModelBinding { @@ -36,6 +46,16 @@ void main() { ref.dispose(); }); + test('maybeWatchCached rethrows non-ViewModelError', () { + final ref = TestRef(); + const key = ThrowingHashKey(); + expect( + () => ref.maybeWatchCached(key: key), + throwsA(isA()), + ); + ref.dispose(); + }); + test('watchCached/maybeWatchCached by key returns same instance', () async { final ref = TestRef(); final provider = ViewModelSpec( @@ -68,6 +88,16 @@ void main() { expect(res, isNull); }); + test('maybeReadCached rethrows non-ViewModelError', () { + final ref = TestRef(); + const key = ThrowingHashKey(); + expect( + () => ref.maybeReadCached(key: key), + throwsA(isA()), + ); + ref.dispose(); + }); + test('readCached throws when missing', () { expect( () => ViewModel.readCached(key: 'missing'), diff --git a/packages/view_model/test/view_model_test.dart b/packages/view_model/test/view_model_test.dart index dadb129..331e2c5 100644 --- a/packages/view_model/test/view_model_test.dart +++ b/packages/view_model/test/view_model_test.dart @@ -43,6 +43,24 @@ class MyViewModelLifecycle extends ViewModelLifecycle { } } +class ThrowingHashKey { + const ThrowingHashKey(); + + @override + int get hashCode => throw StateError('hashCode failure'); + + @override + bool operator ==(Object other) => identical(this, other); +} + +class ReentrantStateVM extends StateViewModel { + ReentrantStateVM() : super(state: 0); + + void increment() { + setState(state + 1); + } +} + void main() { group('view_model lifecycle and features', () { late MyViewModelLifecycle lifecycleObserver; @@ -170,6 +188,14 @@ void main() { ); }); + test('maybeReadCached only swallows ViewModelError', () { + const key = ThrowingHashKey(); + expect( + () => ViewModel.maybeReadCached(key: key), + throwsA(isA()), + ); + }); + test("readCached by tag", () { final factory = InstanceFactory( builder: () => TestStatelessViewModel(), @@ -200,6 +226,43 @@ void main() { expect(updated, isTrue); }); + test('notifyListeners supports removing listeners during iteration', () { + final vm = TestStatelessViewModel(); + int callbackCount = 0; + late Function() removeSelf; + removeSelf = vm.listen(onChanged: () { + callbackCount++; + removeSelf(); + }); + + expect(vm.notifyListeners, returnsNormally); + expect(callbackCount, 1); + expect(vm.hasListeners, isFalse); + }); + + test('state listeners support removing listeners during iteration', () { + final vm = ReentrantStateVM(); + int stateCallbacks = 0; + int normalCallbacks = 0; + + late Function() removeStateListener; + removeStateListener = vm.listenState(onChanged: (_, __) { + stateCallbacks++; + removeStateListener(); + }); + + late Function() removeListener; + removeListener = vm.listen(onChanged: () { + normalCallbacks++; + removeListener(); + }); + + expect(vm.increment, returnsNormally); + expect(stateCallbacks, 1); + expect(normalCallbacks, 1); + expect(vm.hasListeners, isFalse); + }); + test("addDispose", () async { final vm = TestStatelessViewModel(); const bool disposeCalled = false; diff --git a/packages/view_model_annotation/CHANGELOG.md b/packages/view_model_annotation/CHANGELOG.md index a8c52e5..6d0e45b 100644 --- a/packages/view_model_annotation/CHANGELOG.md +++ b/packages/view_model_annotation/CHANGELOG.md @@ -1,3 +1,6 @@ +## 1.0.2 +- Improve release consistency for annotation package. + ## 1.0.1 - Version bump for consistency diff --git a/packages/view_model_annotation/pubspec.yaml b/packages/view_model_annotation/pubspec.yaml index c889407..1c2f4d7 100644 --- a/packages/view_model_annotation/pubspec.yaml +++ b/packages/view_model_annotation/pubspec.yaml @@ -1,6 +1,6 @@ name: view_model_annotation description: Annotation package for view_model code generation. -version: 1.0.1 +version: 1.0.2 homepage: https://github.com/lwj1994/flutter_view_model/tree/main/packages/view_model_annotation repository: https://github.com/lwj1994/flutter_view_model/tree/main/packages/view_model_annotation issue_tracker: https://github.com/lwj1994/flutter_view_model/issues diff --git a/packages/view_model_generator/CHANGELOG.md b/packages/view_model_generator/CHANGELOG.md index 1d26f60..ec8a3d7 100644 --- a/packages/view_model_generator/CHANGELOG.md +++ b/packages/view_model_generator/CHANGELOG.md @@ -1,3 +1,6 @@ +## 1.0.2 +- Improve generator error reporting with explicit source errors. + ## 1.0.1 - Version bump for consistency diff --git a/packages/view_model_generator/example/pubspec.lock b/packages/view_model_generator/example/pubspec.lock index 6842712..5910279 100644 --- a/packages/view_model_generator/example/pubspec.lock +++ b/packages/view_model_generator/example/pubspec.lock @@ -401,21 +401,21 @@ packages: path: "../../view_model" relative: true source: path - version: "1.0.1-dev.2" + version: "1.0.2" view_model_annotation: dependency: "direct overridden" description: path: "../../view_model_annotation" relative: true source: path - version: "1.0.1-dev.2" + version: "1.0.2" view_model_generator: dependency: "direct dev" description: path: ".." relative: true source: path - version: "1.0.1-dev.2" + version: "1.0.2" watcher: dependency: transitive description: diff --git a/packages/view_model_generator/lib/src/spec_generator.dart b/packages/view_model_generator/lib/src/spec_generator.dart index 626eec2..567e03c 100644 --- a/packages/view_model_generator/lib/src/spec_generator.dart +++ b/packages/view_model_generator/lib/src/spec_generator.dart @@ -18,11 +18,19 @@ class ViewModelSpecGenerator extends GeneratorForAnnotation { ConstantReader annotation, BuildStep buildStep, ) { - if (element is! ClassElement) return ''; + if (element is! ClassElement) { + throw InvalidGenerationSourceError( + '@GenSpec can only be applied to classes.', + element: element, + ); + } final className = element.name; if (className == null || className.isEmpty) { - return '// Error: Class name is empty'; + throw InvalidGenerationSourceError( + 'Class name is empty.', + element: element, + ); } final specName = _specVarName(className); @@ -81,7 +89,11 @@ class ViewModelSpecGenerator extends GeneratorForAnnotation { final matchingFactory = _findSpecFactory(element); if (mainCtor == null && matchingFactory == null) { - return '// Skipped: No unnamed constructor or spec factory for $className'; + throw InvalidGenerationSourceError( + '$className must have an unnamed constructor or a factory named ' + '`spec`.', + element: element, + ); } // Collect class-owned params. @@ -134,7 +146,10 @@ class ViewModelSpecGenerator extends GeneratorForAnnotation { } if (argCount > 4) { - return '// Skipped: constructor with > 4 required parameters'; + throw InvalidGenerationSourceError( + '$className has $argCount parameters, but only up to 4 are supported.', + element: element, + ); } // Prepare builder signature diff --git a/packages/view_model_generator/pubspec.lock b/packages/view_model_generator/pubspec.lock index bc8abc0..b881087 100644 --- a/packages/view_model_generator/pubspec.lock +++ b/packages/view_model_generator/pubspec.lock @@ -477,10 +477,10 @@ packages: dependency: "direct main" description: name: view_model_annotation - sha256: "34bcf871134aa5808163539af656b6d1a288876f3ff6a58e9cf8d618451e7985" + sha256: "41ef3b28a35b5a87915d90d0b746aee59d25c54d0a2b6efbb0eec501e35e0dbe" url: "https://pub.dev" source: hosted - version: "1.0.1" + version: "1.0.2" vm_service: dependency: transitive description: diff --git a/packages/view_model_generator/pubspec.yaml b/packages/view_model_generator/pubspec.yaml index 90078bc..16796ef 100644 --- a/packages/view_model_generator/pubspec.yaml +++ b/packages/view_model_generator/pubspec.yaml @@ -1,6 +1,6 @@ name: view_model_generator description: Automatically generate ViewModelSpec code from @GenSpec annotations. Reduces boilerplate for the view_model package. -version: 1.0.1 +version: 1.0.2 homepage: https://github.com/lwj1994/flutter_view_model/tree/main/packages/view_model_generator repository: https://github.com/lwj1994/flutter_view_model/tree/main/packages/view_model_generator issue_tracker: https://github.com/lwj1994/flutter_view_model/issues @@ -13,7 +13,7 @@ dependencies: analyzer: ^8.1.1 source_gen: ^4.0.0 build: ^4.0.0 - view_model_annotation: 1.0.1 + view_model_annotation: 1.0.2 dev_dependencies: build_runner: ^2.7.1