Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIP-2379 Memory leak audit #75

Merged
merged 4 commits into from
Jun 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .analysis_options
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/src/component/abstract_transition.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -298,6 +301,7 @@ abstract class AbstractTransitionComponent<T extends AbstractTransitionProps, S

var _isUnmounted = false;

@mustCallSuper
@override
void componentWillUnmount() {
_isUnmounted = true;
Expand Down
3 changes: 3 additions & 0 deletions lib/src/component/resize_sensor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion test/over_react/component/resize_sensor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void main() {

action();

await resizesDone;
await resizesDone.then((_) { resizes.close(); });
}

/// Expect resize sensor invokes registered `onInitialize` callback.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions test/test_util/dom_util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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]);

Expand Down
4 changes: 2 additions & 2 deletions test/wsd_test_util/common_component_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down