From aeeb7c4ee933bf936f364c20d6d1c7215a2cb8de Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 29 May 2019 14:58:29 +1000 Subject: [PATCH] fix: always unsubscribe backend when connector reconnects (#1355) --- packages/react-dnd/src/SourceConnector.ts | 36 ++++++++------ packages/react-dnd/src/TargetConnector.ts | 17 ++++--- .../src/__tests__/SourceConnector.spec.tsx | 48 +++++++++++++++++++ .../src/__tests__/TargetConnector.spec.tsx | 26 ++++++++++ 4 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 packages/react-dnd/src/__tests__/SourceConnector.spec.tsx create mode 100644 packages/react-dnd/src/__tests__/TargetConnector.spec.tsx diff --git a/packages/react-dnd/src/SourceConnector.ts b/packages/react-dnd/src/SourceConnector.ts index b007b4cdf6..a5c0e38424 100644 --- a/packages/react-dnd/src/SourceConnector.ts +++ b/packages/react-dnd/src/SourceConnector.ts @@ -93,18 +93,22 @@ export default class SourceConnector implements Connector { } private reconnectDragSource() { - const dragSource = this.dragSource - if (!this.handlerId || !dragSource) { - return - } - // if nothing has changed then don't resubscribe - if ( + const didChange = this.didHandlerIdChange() || this.didConnectedDragSourceChange() || this.didDragSourceOptionsChange() - ) { + + if (didChange) { this.disconnectDragSource() + } + + const dragSource = this.dragSource + if (!this.handlerId || !dragSource) { + return + } + + if (didChange) { this.lastConnectedHandlerId = this.handlerId this.lastConnectedDragSource = dragSource this.lastConnectedDragSourceOptions = this.dragSourceOptions @@ -117,18 +121,22 @@ export default class SourceConnector implements Connector { } private reconnectDragPreview() { - const dragPreview = this.dragPreview - if (!this.handlerId || !dragPreview) { - return - } - // if nothing has changed then don't resubscribe - if ( + const didChange = this.didHandlerIdChange() || this.didConnectedDragPreviewChange() || this.didDragPreviewOptionsChange() - ) { + + if (didChange) { this.disconnectDragPreview() + } + + const dragPreview = this.dragPreview + if (!this.handlerId || !dragPreview) { + return + } + + if (didChange) { this.lastConnectedHandlerId = this.handlerId this.lastConnectedDragPreview = dragPreview this.lastConnectedDragPreviewOptions = this.dragPreviewOptions diff --git a/packages/react-dnd/src/TargetConnector.ts b/packages/react-dnd/src/TargetConnector.ts index ea8094c766..a9f8c62808 100644 --- a/packages/react-dnd/src/TargetConnector.ts +++ b/packages/react-dnd/src/TargetConnector.ts @@ -38,17 +38,22 @@ export default class TargetConnector implements Connector { } public reconnect() { - const dropTarget = this.dropTarget - if (!this.handlerId || !dropTarget) { - return - } // if nothing has changed then don't resubscribe - if ( + const didChange = this.didHandlerIdChange() || this.didDropTargetChange() || this.didOptionsChange() - ) { + + if (didChange) { this.disconnectDropTarget() + } + + const dropTarget = this.dropTarget + if (!this.handlerId || !dropTarget) { + return + } + + if (didChange) { this.lastConnectedHandlerId = this.handlerId this.lastConnectedDropTarget = dropTarget this.lastConnectedDropTargetOptions = this.dropTargetOptions diff --git a/packages/react-dnd/src/__tests__/SourceConnector.spec.tsx b/packages/react-dnd/src/__tests__/SourceConnector.spec.tsx new file mode 100644 index 0000000000..8e1ab28a0e --- /dev/null +++ b/packages/react-dnd/src/__tests__/SourceConnector.spec.tsx @@ -0,0 +1,48 @@ +import SourceConnector from '../SourceConnector' +import { Backend } from 'dnd-core' + +describe('SourceConnector', () => { + let backend: jest.Mocked + let connector: SourceConnector + + beforeEach(() => { + backend = { + setup: jest.fn(), + teardown: jest.fn(), + connectDragSource: jest.fn(), + connectDragPreview: jest.fn(), + connectDropTarget: jest.fn(), + } + connector = new SourceConnector(backend) + }) + + it('unsubscribes drag source when clearing handler id', () => { + const unsubscribeDragSource = jest.fn() + backend.connectDragSource.mockReturnValueOnce(unsubscribeDragSource) + + connector.receiveHandlerId('test') + connector.hooks.dragSource()({}) + expect(backend.connectDragSource).toHaveBeenCalled() + expect(unsubscribeDragSource).not.toHaveBeenCalled() + backend.connectDragSource.mockClear() + + connector.receiveHandlerId(null) + expect(backend.connectDragSource).not.toHaveBeenCalled() + expect(unsubscribeDragSource).toHaveBeenCalled() + }) + + it('unsubscribes drag preview when clearing handler id', () => { + const unsubscribeDragPreview = jest.fn() + backend.connectDragPreview.mockReturnValueOnce(unsubscribeDragPreview) + + connector.receiveHandlerId('test') + connector.hooks.dragPreview()({}) + expect(backend.connectDragPreview).toHaveBeenCalled() + expect(unsubscribeDragPreview).not.toHaveBeenCalled() + backend.connectDragPreview.mockClear() + + connector.receiveHandlerId(null) + expect(backend.connectDragPreview).not.toHaveBeenCalled() + expect(unsubscribeDragPreview).toHaveBeenCalled() + }) +}) diff --git a/packages/react-dnd/src/__tests__/TargetConnector.spec.tsx b/packages/react-dnd/src/__tests__/TargetConnector.spec.tsx new file mode 100644 index 0000000000..502f7b5b81 --- /dev/null +++ b/packages/react-dnd/src/__tests__/TargetConnector.spec.tsx @@ -0,0 +1,26 @@ +import TargetConnector from '../TargetConnector' + +describe('TargetConnector', () => { + it('unsubscribes drop target when clearing handler id', () => { + const backend = { + setup: jest.fn(), + teardown: jest.fn(), + connectDragSource: jest.fn(), + connectDragPreview: jest.fn(), + connectDropTarget: jest.fn(), + } + const connector = new TargetConnector(backend) + const unsubscribeDropTarget = jest.fn() + backend.connectDropTarget.mockReturnValueOnce(unsubscribeDropTarget) + + connector.receiveHandlerId('test') + connector.hooks.dropTarget()({}) + expect(backend.connectDropTarget).toHaveBeenCalled() + expect(unsubscribeDropTarget).not.toHaveBeenCalled() + backend.connectDropTarget.mockClear() + + connector.receiveHandlerId(null) + expect(backend.connectDropTarget).not.toHaveBeenCalled() + expect(unsubscribeDropTarget).toHaveBeenCalled() + }) +})