Skip to content

Commit

Permalink
Fixing memory leak in DragDropContext. Fixes #715 (#720)
Browse files Browse the repository at this point in the history
Fixes #715
  • Loading branch information
alexreardon authored Aug 19, 2018
1 parent b939e9a commit 48e0d84
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/view/drag-drop-context/drag-drop-context.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export default class DragDropContext extends React.Component<Props> {
}

componentWillUnmount() {
window.addEventListener('error', this.onWindowError);
window.removeEventListener('error', this.onWindowError);

const state: State = this.store.getState();
if (state.phase !== 'IDLE') {
Expand Down
8 changes: 5 additions & 3 deletions src/view/drag-handle/util/create-post-drag-event-preventer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
/* eslint-disable no-use-before-define */
import type { EventBinding } from './event-types';
import type { EventBinding, EventOptions } from './event-types';
import { bindEvents, unbindEvents } from './bind-events';

type GetWindowFn = () => HTMLElement;
Expand All @@ -10,6 +10,8 @@ export type EventPreventer = {|
abort: () => void,
|};

const sharedOptions: EventOptions = { capture: true };

export default (getWindow: GetWindowFn): EventPreventer => {
let isBound: boolean = false;

Expand All @@ -18,15 +20,15 @@ export default (getWindow: GetWindowFn): EventPreventer => {
return;
}
isBound = true;
bindEvents(getWindow(), pointerEvents, { capture: true });
bindEvents(getWindow(), pointerEvents, sharedOptions);
};

const unbind = () => {
if (!isBound) {
return;
}
isBound = false;
unbindEvents(getWindow(), pointerEvents, { capture: true });
unbindEvents(getWindow(), pointerEvents, sharedOptions);
};

const pointerEvents: EventBinding[] = [
Expand Down
15 changes: 0 additions & 15 deletions test/unit/view/drag-drop-context/lifecycle.spec.js

This file was deleted.

35 changes: 35 additions & 0 deletions test/unit/view/drag-drop-context/unmount.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// @flow
import React from 'react';
import { mount } from 'enzyme';
import App from './app';
import DragDropContext from '../../../../src/view/drag-drop-context';

it('should not throw when unmounting', () => {
const wrapper = mount(
<DragDropContext onDragEnd={() => {}}>
<App />
</DragDropContext>,
);

expect(() => wrapper.unmount()).not.toThrow();
});

it('should clean up any window event handlers', () => {
jest.spyOn(window, 'addEventListener');
jest.spyOn(window, 'removeEventListener');

const wrapper = mount(
<DragDropContext onDragEnd={() => {}}>
<App />
</DragDropContext>,
);

wrapper.unmount();

expect(window.addEventListener.mock.calls).toHaveLength(
window.removeEventListener.mock.calls.length,
);
// validation
expect(window.addEventListener).toHaveBeenCalled();
expect(window.removeEventListener).toHaveBeenCalled();
});

0 comments on commit 48e0d84

Please sign in to comment.