Merge pull request #75 from aaronlademann-wf/memory-leak-audit

UIP-2379 Memory leak audit
Workiva · Jun 13, 2017 · 7d225fc · 7d225fc
2 parents 61fbe7a + 43aa8b1
commit 7d225fc
Show file tree

Hide file tree

Showing 8 changed files with 17 additions and 7 deletions.
diff --git a/.analysis_options b/.analysis_options
@@ -11,6 +11,8 @@ linter:
     - avoid_init_to_null
     - avoid_return_types_on_setters
     - camel_case_types
+    - cancel_subscriptions
+    - close_sinks
     - empty_constructor_bodies
     - hash_and_equals
     - library_names

diff --git a/lib/src/component/abstract_transition.dart b/lib/src/component/abstract_transition.dart
@@ -17,6 +17,7 @@ library abstract_transition;
 import 'dart:async';
 import 'dart:html';
 
+import 'package:meta/meta.dart';
 import 'package:over_react/over_react.dart';
 
 @AbstractProps()
@@ -175,6 +176,7 @@ abstract class AbstractTransitionComponent<T extends AbstractTransitionProps, S
 
   /// Listens for the next `transitionend` event and invokes a callback after
   /// the event is dispatched.
+  @mustCallSuper
   void onNextTransitionEnd(complete()) {
     var skipCount = props.transitionCount - 1;
 
@@ -262,6 +264,7 @@ abstract class AbstractTransitionComponent<T extends AbstractTransitionProps, S
   /// component than to break state changes waiting for a transition that will never happen.
   bool _transitionNotGuaranteed = false;
 
+  @mustCallSuper
   @override
   void componentDidUpdate(Map prevProps, Map prevState) {
     _transitionNotGuaranteed = false;
@@ -298,6 +301,7 @@ abstract class AbstractTransitionComponent<T extends AbstractTransitionProps, S
 
   var _isUnmounted = false;
 
+  @mustCallSuper
   @override
   void componentWillUnmount() {
     _isUnmounted = true;

diff --git a/lib/src/component/resize_sensor.dart b/lib/src/component/resize_sensor.dart
@@ -19,6 +19,7 @@ library resize_sensor;
 import 'dart:collection';
 import 'dart:html';
 
+import 'package:meta/meta.dart';
 import 'package:platform_detect/platform_detect.dart';
 import 'package:react/react.dart' as react;
 import 'package:over_react/over_react.dart';
@@ -102,13 +103,15 @@ class ResizeSensorComponent extends UiComponent<ResizeSensorProps> with _SafeAni
     ..addProps(ResizeSensorPropsMixin.defaultProps)
   );
 
+  @mustCallSuper
   @override
   void componentWillUnmount() {
     super.componentWillUnmount();
 
     cancelAnimationFrames();
   }
 
+  @mustCallSuper
   @override
   void componentDidMount() {
     if (props.quickMount) {

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -16,10 +16,9 @@ dependencies:
   source_span: "^1.2.0"
   transformer_utils: "^0.1.1"
   w_flux: "^2.5.0"
-  platform_detect: "^1.1.1"
+  platform_detect: "^1.3.2"
   quiver: ">=0.21.4 <0.25.0"
 dev_dependencies:
-  browser_detect: "^1.0.4"
   matcher: ">=0.11.0 <0.13.0"
   coverage: "^0.7.2"
   dart_dev: "^1.0.5"

diff --git a/test/over_react/component/resize_sensor_test.dart b/test/over_react/component/resize_sensor_test.dart
@@ -112,7 +112,7 @@ void main() {
 
       action();
 
-      await resizesDone;
+      await resizesDone.then((_) { resizes.close(); });
     }
 
     /// Expect resize sensor invokes registered `onInitialize` callback.

diff --git a/test/over_react/component_declaration/flux_component_test.dart b/test/over_react/component_declaration/flux_component_test.dart
@@ -152,6 +152,8 @@ void main() {
       await nextTick();
       expect(numberOfCalls, 1,
           reason: 'the subscription to have been cancelled, and should not receive additional events');
+
+      controller.close();
     });
 
     test('should not redraw after being unmounted', () async {

diff --git a/test/test_util/dom_util.dart b/test/test_util/dom_util.dart
@@ -18,7 +18,7 @@ import 'dart:async';
 import 'dart:html';
 import 'dart:js';
 
-import 'package:browser_detect/browser_detect.dart';
+import 'package:platform_detect/platform_detect.dart';
 
 /// Dispatches a `transitionend` event when the CSS transition of the [element]
 /// is complete. Returns a [Future] that completes after the event has been dispatched.
@@ -43,7 +43,7 @@ Future triggerTransitionEnd(Element element) {
   }
 
   // Need to use webkitTransitionEnd in Edge. See https://github.com/dart-lang/sdk/issues/26972
-  var eventName = (browser.isIe && browser.version > '11') ? 'webkitTransitionEnd' : 'transitionend';
+  var eventName = (browser.isInternetExplorer && browser.version.major > 11) ? 'webkitTransitionEnd' : 'transitionend';
 
   jsEvent.callMethod('initEvent', [eventName, true, true]);
 

diff --git a/test/wsd_test_util/common_component_tests.dart b/test/wsd_test_util/common_component_tests.dart
@@ -23,7 +23,7 @@ import 'dart:html';
 ])
 import 'dart:mirrors';
 
-import 'package:browser_detect/browser_detect.dart';
+import 'package:platform_detect/platform_detect.dart';
 import 'package:over_react/over_react.dart';
 import 'package:over_react/src/component_declaration/component_base.dart' as component_base;
 import 'package:react/react_client.dart';
@@ -434,7 +434,7 @@ void commonComponentTests(BuilderOnlyUiFactory factory, {
   skippedPropKeys = flatten(skippedPropKeys).toList();
 
   // TODO: Remove this short-circuit when UIP-1125.
-  if (browser.isIe && browser.version <= '10') return;
+  if (browser.isInternetExplorer && browser.version.major <= 10) return;
 
   if (shouldTestPropForwarding) {
     testPropForwarding(factory, childrenFactory,