From 94262e247cc66d49db338352eb4beb5ca0272b5e Mon Sep 17 00:00:00 2001 From: Ben Campbell Date: Wed, 28 Jun 2017 14:42:49 -0700 Subject: [PATCH 1/5] UIP-2161 Implement DisposableManagerv3 for UiComponent All components now implement the DisposableManagerV3 interface proxying management calls to an internal instance of Disposable. This provides consumers with the set of w_common Disposable methods to ease the management of various object types. The disposal of these managed objects is triggered by the unmounting of the compoment. --- lib/src/component/abstract_transition.dart | 1 + .../component_declaration/component_base.dart | 70 ++++++++++++- .../component_declaration/flux_component.dart | 4 + pubspec.yaml | 1 + .../component_base_test.dart | 98 +++++++++++++++++++ 5 files changed, 173 insertions(+), 1 deletion(-) diff --git a/lib/src/component/abstract_transition.dart b/lib/src/component/abstract_transition.dart index 344c3ba30..18b791ee7 100644 --- a/lib/src/component/abstract_transition.dart +++ b/lib/src/component/abstract_transition.dart @@ -289,6 +289,7 @@ abstract class AbstractTransitionComponent(); /// /// Extends [react.Component]. /// +/// Implements [DisposableManagerV3] +/// /// Related: [UiStatefulComponent] -abstract class UiComponent extends react.Component { +abstract class UiComponent extends react.Component implements DisposableManagerV3 { + Disposable _disposableProxy; + /// The props for the non-forwarding props defined in this component. Iterable get consumedProps => null; @@ -156,6 +163,12 @@ abstract class UiComponent extends react.Component { validateRequiredProps(props); } + @override + @mustCallSuper + void componentWillUnmount() { + _disposableProxy?.dispose(); + } + // ---------------------------------------------------------------------- // ---------------------------------------------------------------------- @@ -199,6 +212,61 @@ abstract class UiComponent extends react.Component { // END Typed props helpers // ---------------------------------------------------------------------- // ---------------------------------------------------------------------- + + // ---------------------------------------------------------------------- + // ---------------------------------------------------------------------- + // BEGIN DisposableManagerV3 interface implementation + // + + @override + Future awaitBeforeDispose(Future future) => + _getDisposableProxy().awaitBeforeDispose(future); + + @override + Future getManagedDelayedFuture(Duration duration, T callback()) => + _getDisposableProxy().getManagedDelayedFuture(duration, callback); + + @override + Timer getManagedPeriodicTimer(Duration duration, void callback(Timer timer)) => + _getDisposableProxy().getManagedPeriodicTimer(duration, callback); + + @override + Timer getManagedTimer(Duration duration, void callback()) => + _getDisposableProxy().getManagedTimer(duration, callback); + + @override + Completer manageCompleter(Completer completer) => + _getDisposableProxy().manageCompleter(completer); + + @override + void manageDisposable(Disposable disposable) => + _getDisposableProxy().manageDisposable(disposable); + + @override + void manageDisposer(Disposer disposer) => + _getDisposableProxy().manageDisposer(disposer); + + @override + void manageStreamController(StreamController controller) => + _getDisposableProxy().manageStreamController(controller); + + @override + void manageStreamSubscription(StreamSubscription subscription) => + _getDisposableProxy().manageStreamSubscription(subscription); + + /// Instantiates a new [Disposable] instance on the first call to the + /// [DisposableManagerV3] method. + Disposable _getDisposableProxy() { + if (_disposableProxy == null) { + _disposableProxy = new Disposable(); + } + return _disposableProxy; + } + + // + // END Typed props helpers + // ---------------------------------------------------------------------- + // ---------------------------------------------------------------------- } /// The basis for a stateful over_react component. diff --git a/lib/src/component_declaration/flux_component.dart b/lib/src/component_declaration/flux_component.dart index c34815d20..0e3614fb9 100644 --- a/lib/src/component_declaration/flux_component.dart +++ b/lib/src/component_declaration/flux_component.dart @@ -89,6 +89,8 @@ abstract class FluxUiComponent extends UiComponent=0.21.4 <0.26.0" diff --git a/test/over_react/component_declaration/component_base_test.dart b/test/over_react/component_declaration/component_base_test.dart index b4940ce65..f07309f18 100644 --- a/test/over_react/component_declaration/component_base_test.dart +++ b/test/over_react/component_declaration/component_base_test.dart @@ -14,6 +14,7 @@ library over_react.component_declaration.component_base_test; +import 'dart:async'; import 'dart:html'; import 'package:over_react/over_react.dart' show Dom, DummyComponent, ValidationUtil; @@ -21,6 +22,7 @@ import 'package:over_react/src/component_declaration/component_base.dart'; import 'package:over_react/src/component_declaration/component_type_checking.dart'; import 'package:react/react_client.dart'; import 'package:test/test.dart'; +import 'package:w_common/disposable.dart'; import '../../test_util/test_util.dart'; import '../../wsd_test_util/validation_util_helpers.dart'; @@ -623,6 +625,102 @@ main() { expect(classes.toClassName(), equals('class-1')); expect(classes.toClassNameBlacklist(), equals('blacklist-1')); }); + + group('on unmount', () { + TestComponentComponent component; + ReactElement instance; + Duration longDuration = const Duration(milliseconds: 200); + Duration shortDuration = const Duration(milliseconds: 100); + + setUp(() { + instance = render(TestComponent()()); + component = getDartComponent(instance); + }); + + Future unmountAndDisposal() async { + unmount(instance); + // Provide timers a window to fire + await new Future.delayed(longDuration); + } + + test('should complete delayed Future with ObjectDisposedException', () async { + expect(component.getManagedDelayedFuture(shortDuration, + expectAsync0(() {}, count: 0, reason: 'Did not expect callback to be invoked.')), + throwsA(new isInstanceOf())); + + await unmountAndDisposal(); + }); + + test('should cancel periodic timer', () async { + var timer = component.getManagedPeriodicTimer(shortDuration, + expectAsync1((_) {}, count: 0, reason: 'Did not expect callback to be invoked.')); + + expect(timer.isActive, isTrue); + await unmountAndDisposal(); + expect(timer.isActive, isFalse); + }); + + test('should cancel timer', () async { + var timer = component.getManagedTimer(shortDuration, + expectAsync1((_){}, count: 0, reason: 'Did not expect callback to be invoked.')); + + expect(timer.isActive, isTrue); + await unmountAndDisposal(); + expect(timer.isActive, isFalse); + }); + + test('should complete uncompleted managed Completer with ObjectDisposedException', () async { + var completer = new Completer(); + component.manageCompleter(completer); + completer.future.catchError(expectAsync1((Object err) => + expect(err, new isInstanceOf()))); + + expect(completer.isCompleted, isFalse); + await unmountAndDisposal(); + expect(completer.isCompleted, isTrue); + }); + + test('should dispose managed Disposable', () async { + var disposable = new Disposable(); + component.manageDisposable(disposable); + expect(disposable.isDisposed, isFalse); + await unmountAndDisposal(); + expect(disposable.isDisposed, isTrue); + }); + + test('should call managed disposers', () async { + var disposerCalled = false; + component.manageDisposer(() async => disposerCalled = true); + expect(disposerCalled, isFalse); + await unmountAndDisposal(); + expect(disposerCalled, isTrue); + }); + + test('should close managed StreamController', () async { + //ignore: close_sinks + var streamController = new StreamController.broadcast(); + component.manageStreamController(streamController); + expect(streamController.isClosed, isFalse); + await unmountAndDisposal(); + expect(streamController.isClosed, isTrue); + }); + + test('should cancel managed StreamSubscription', () async{ + var streamController = new StreamController.broadcast(); + // ignore: cancel_subscriptions + var streamSubscription = streamController.stream + .listen(expectAsync1((_) {}, + count: 0, + reason: 'Did not expect event after cancelling subscription')); + + component.manageStreamSubscription(streamSubscription); + await unmountAndDisposal(); + + streamController + ..add(null) + ..close(); + }); + }, timeout: new Timeout(const Duration(milliseconds: 250))); }); group('UiStatefulComponent', () { From 6374a6019aa4bf93522e3746c4c7e6140f2ce809 Mon Sep 17 00:00:00 2001 From: Ben Campbell Date: Thu, 29 Jun 2017 07:50:00 -0700 Subject: [PATCH 2/5] UIP-2161 Use correct identifier in section comment --- lib/src/component_declaration/component_base.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/component_declaration/component_base.dart b/lib/src/component_declaration/component_base.dart index 1afedb895..22ef61e64 100644 --- a/lib/src/component_declaration/component_base.dart +++ b/lib/src/component_declaration/component_base.dart @@ -316,7 +316,7 @@ abstract class UiStatefulComponent typedStateFactory({}); // - // END Typed state helpers + // END DisposableManagerV3 interface implementation // ---------------------------------------------------------------------- // ---------------------------------------------------------------------- } From d62e5e35e4b0907dadb9975fb30a59756bfa7363 Mon Sep 17 00:00:00 2001 From: Ben Campbell Date: Thu, 29 Jun 2017 08:38:04 -0700 Subject: [PATCH 3/5] UIP-2161 Add test for awaited futures --- .../component_base_test.dart | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/over_react/component_declaration/component_base_test.dart b/test/over_react/component_declaration/component_base_test.dart index f07309f18..da30885ec 100644 --- a/test/over_react/component_declaration/component_base_test.dart +++ b/test/over_react/component_declaration/component_base_test.dart @@ -643,6 +643,29 @@ main() { await new Future.delayed(longDuration); } + test('should await future before disposing', () async { + // ignore: close_sinks + var streamController = new StreamController.broadcast(); + var completer = new Completer(); + + // Manage pending future + component.awaitBeforeDispose(completer.future); + + // Add events to stream + component.manageDisposer(() async => streamController.add('disposalFuture')); + completer.future.then(streamController.add); + + // Perform events out of order + await unmountAndDisposal(); + completer.complete('awaitedFuture'); + + // Ensure events resolve in the correct order + expect(streamController.stream, emitsInOrder([ + 'awaitedFuture', + 'disposalFuture', + ])); + }); + test('should complete delayed Future with ObjectDisposedException', () async { expect(component.getManagedDelayedFuture(shortDuration, expectAsync0(() {}, count: 0, reason: 'Did not expect callback to be invoked.')), From 542bd57f1eda049c17ae15cd57a673b23bb43a1f Mon Sep 17 00:00:00 2001 From: Ben Campbell Date: Thu, 29 Jun 2017 08:46:36 -0700 Subject: [PATCH 4/5] UIP-2161 Revert section comment changes --- lib/src/component_declaration/component_base.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/component_declaration/component_base.dart b/lib/src/component_declaration/component_base.dart index 22ef61e64..06de64795 100644 --- a/lib/src/component_declaration/component_base.dart +++ b/lib/src/component_declaration/component_base.dart @@ -264,7 +264,7 @@ abstract class UiComponent extends react.Component imple } // - // END Typed props helpers + // END DisposableManagerV3 interface implementation // ---------------------------------------------------------------------- // ---------------------------------------------------------------------- } @@ -316,7 +316,7 @@ abstract class UiStatefulComponent typedStateFactory({}); // - // END DisposableManagerV3 interface implementation + // END Typed props helpers // ---------------------------------------------------------------------- // ---------------------------------------------------------------------- } From c8a2e066da77cbc7786d958caa1325c424c6f2c7 Mon Sep 17 00:00:00 2001 From: Ben Campbell Date: Thu, 29 Jun 2017 11:29:41 -0700 Subject: [PATCH 5/5] UIP-2161 Address copypasta mistake --- lib/src/component_declaration/component_base.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/component_declaration/component_base.dart b/lib/src/component_declaration/component_base.dart index 06de64795..e2093f35a 100644 --- a/lib/src/component_declaration/component_base.dart +++ b/lib/src/component_declaration/component_base.dart @@ -316,7 +316,7 @@ abstract class UiStatefulComponent typedStateFactory({}); // - // END Typed props helpers + // END Typed state helpers // ---------------------------------------------------------------------- // ---------------------------------------------------------------------- }