fix: only call *onDrag callbacks when dragging files (#619)

BREAKING CHANGE:  Callbacks won't get executed for non-file items anymore i.e. if items aren't of type `File` when react-dropzone will ignore them.

src/index.js

@@ -10,7 +10,8 @@ import {
   fileMatchSize,
   onDocumentDragOver,
   getDataTransferItems as defaultGetDataTransferItem,
-  isIeOrEdge
+  isIeOrEdge,
+  hasFiles
 } from './utils'
 import styles from './utils/styles'
 
@@ -84,9 +85,11 @@ class Dropzone extends React.Component {
   }
 
   onDragStart(evt) {
-    if (this.props.onDragStart) {
-      this.props.onDragStart.call(this, evt)
-    }
+    Promise.resolve(this.props.getDataTransferItems(evt)).then(draggedFiles => {
+      if (hasFiles(draggedFiles) && this.props.onDragStart) {
+        this.props.onDragStart.call(this, evt)
+      }
+    })
   }
 
   onDragEnter(evt) {
@@ -100,14 +103,17 @@ class Dropzone extends React.Component {
     evt.persist()
 
     Promise.resolve(this.props.getDataTransferItems(evt)).then(draggedFiles => {
-      this.setState({
-        isDragActive: true, // Do not rely on files for the drag state. It doesn't work in Safari.
-        draggedFiles
-      })
+      if (hasFiles(draggedFiles)) {
+        this.setState({
+          isDragActive: true, // Do not rely on files for the drag state. It doesn't work in Safari.
+          draggedFiles
+        })
+
+        if (this.props.onDragEnter) {
+          this.props.onDragEnter.call(this, evt)
+        }
+      }
     })
-    if (this.props.onDragEnter) {
-      this.props.onDragEnter.call(this, evt)
-    }
   }
 
   onDragOver(evt) {
@@ -123,9 +129,12 @@ class Dropzone extends React.Component {
       // continue regardless of error
     }
 
-    if (this.props.onDragOver) {
-      this.props.onDragOver.call(this, evt)
-    }
+    Promise.resolve(this.props.getDataTransferItems(evt)).then(draggedFiles => {
+      if (hasFiles(draggedFiles) && this.props.onDragOver) {
+        this.props.onDragOver.call(this, evt)
+      }
+    })
+
     return false
   }
 
@@ -144,9 +153,11 @@ class Dropzone extends React.Component {
       draggedFiles: []
     })
 
-    if (this.props.onDragLeave) {
-      this.props.onDragLeave.call(this, evt)
-    }
+    Promise.resolve(this.props.getDataTransferItems(evt)).then(draggedFiles => {
+      if (hasFiles(draggedFiles) && this.props.onDragLeave) {
+        this.props.onDragLeave.call(this, evt)
+      }
+    })
   }
 
   onDrop(evt) {
@@ -210,7 +221,7 @@ class Dropzone extends React.Component {
         rejectedFiles.push(...acceptedFiles.splice(0))
       }
 
-      if (onDrop) {
+      if (hasFiles(fileList) && onDrop) {
         onDrop.call(this, acceptedFiles, rejectedFiles, evt)
       }
 

src/index.spec.js

@@ -15,7 +15,18 @@ const flushPromises = wrapper =>
 const Dropzone = require('./index')
 const DummyChildComponent = () => null
 
+const createFile = (name, size, type) => {
+  const file = new File([], name, { type })
+  Object.defineProperty(file, 'size', {
+    get() {
+      return size
+    }
+  })
+  return file
+}
+
 let files
+let nonFileItems
 let images
 
 const rejectColor = 'red'
@@ -33,26 +44,19 @@ const acceptStyle = {
 
 describe('Dropzone', () => {
   beforeEach(() => {
-    files = [
-      {
-        name: 'file1.pdf',
-        size: 1111,
-        type: 'application/pdf'
-      }
-    ]
+    files = [createFile('file1.pdf', 1111, 'application/pdf')]
 
-    images = [
-      {
-        name: 'cats.gif',
-        size: 1234,
-        type: 'image/gif'
-      },
+    nonFileItems = [
       {
-        name: 'dogs.jpg',
-        size: 2345,
-        type: 'image/jpeg'
+        kind: 'string',
+        type: 'text/plain',
+        getAsFile() {
+          return null
+        }
       }
     ]
+
+    images = [createFile('cats.gif', 1234, 'image/gif'), createFile('dogs.gif', 2345, 'image/jpeg')]
   })
 
   describe('basics', () => {
@@ -298,26 +302,34 @@ describe('Dropzone', () => {
     })
   })
 
-  describe('drag-n-drop', () => {
-    it('should override onDrag* methods', () => {
+  describe('drag-n-drop', async () => {
+    it('should override onDrag* methods', async () => {
       const props = {
         onDragStart: jest.fn(),
         onDragEnter: jest.fn(),
         onDragOver: jest.fn(),
         onDragLeave: jest.fn()
       }
       const component = mount(<Dropzone {...props} />)
-      component.simulate('dragStart')
+
+      component.simulate('dragStart', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragStart).toHaveBeenCalled()
-      component.simulate('dragEnter', { dataTransfer: { items: files } })
+
+      await component.simulate('dragEnter', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragEnter).toHaveBeenCalled()
-      component.simulate('dragOver', { dataTransfer: { items: files } })
+
+      await component.simulate('dragOver', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragOver).toHaveBeenCalled()
-      component.simulate('dragLeave', { dataTransfer: { items: files } })
+
+      await component.simulate('dragLeave', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragLeave).toHaveBeenCalled()
     })
 
-    it('should guard dropEffect in onDragOver for IE', () => {
+    it('should guard dropEffect in onDragOver for IE', async () => {
       const props = {
         onDragStart: jest.fn(),
         onDragEnter: jest.fn(),
@@ -326,42 +338,86 @@ describe('Dropzone', () => {
       const component = mount(<Dropzone {...props} />)
 
       // Using Proxy we'll emulate IE throwing when setting dataTransfer.dropEffect
-      const eventProxy = new Proxy(
-        {},
-        {
-          get: (target, prop) => {
-            switch (prop) {
-              case 'dataTransfer':
-                throw new Error('IE does not support rrror')
-              default:
-                return function noop() {}
+      const eventProxy = {
+        preventDefault() {},
+        stopPropagation() {},
+        dataTransfer: new Proxy(
+          {},
+          {
+            set: (target, prop) => {
+              switch (prop) {
+                case 'dropEffect':
+                  throw new Error('IE does not support setting {dropEffect}')
+                default:
+                  break
+              }
             }
           }
-        }
-      )
+        )
+      }
 
       // And using then we'll call the onDragOver with the proxy instead of event
-      const componentOnDragOver = component.instance().onDragOver
+      const instance = component.instance()
+      const componentOnDragOver = instance.onDragOver
       const onDragOver = jest
-        .spyOn(component.instance(), 'onDragOver')
+        .spyOn(instance, 'onDragOver')
         .mockImplementation(() => componentOnDragOver(eventProxy))
 
-      component.simulate('dragStart', { dataTransfer: { items: files } })
+      component.simulate('dragStart', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragStart).toHaveBeenCalled()
-      component.simulate('dragEnter', { dataTransfer: { items: files } })
+
+      component.simulate('dragEnter', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragEnter).toHaveBeenCalled()
-      component.simulate('dragLeave', { dataTransfer: { items: files } })
+
+      component.simulate('dragLeave', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(props.onDragLeave).toHaveBeenCalled()
+
       // It should not throw the error
-      component.simulate('dragOver', { dataTransfer: { items: files } })
+      component.simulate('dragOver', { dataTransfer: { files } })
+      await flushPromises(component)
       expect(onDragOver).not.toThrow()
     })
 
+    it('should not call onDrag* if there are no files', async () => {
+      const props = {
+        onDragStart: jest.fn(),
+        onDragEnter: jest.fn(),
+        onDragOver: jest.fn(),
+        onDragLeave: jest.fn(),
+        onDrop: jest.fn()
+      }
+
+      const component = mount(<Dropzone {...props} />)
+
+      component.simulate('dragStart', { dataTransfer: { items: nonFileItems } })
+      await flushPromises(component)
+      expect(props.onDragStart).not.toHaveBeenCalled()
+
+      component.simulate('dragEnter', { dataTransfer: { items: nonFileItems } })
+      await flushPromises(component)
+      expect(props.onDragEnter).not.toHaveBeenCalled()
+
+      component.simulate('dragOver', { dataTransfer: { items: nonFileItems } })
+      await flushPromises(component)
+      expect(props.onDragOver).not.toHaveBeenCalled()
+
+      component.simulate('dragLeave', { dataTransfer: { items: nonFileItems } })
+      await flushPromises(component)
+      expect(props.onDragLeave).not.toHaveBeenCalled()
+
+      component.simulate('drop', { dataTransfer: { items: nonFileItems } })
+      await flushPromises(component)
+      expect(props.onDrop).not.toHaveBeenCalled()
+    })
+
     it('should set proper dragActive state on dragEnter', async () => {
-      const dropzone = mount(<Dropzone>{props => <DummyChildComponent {...props} />}</Dropzone>)
-      dropzone.simulate('dragEnter', { dataTransfer: { files } })
+      const component = mount(<Dropzone>{props => <DummyChildComponent {...props} />}</Dropzone>)
+      component.simulate('dragEnter', { dataTransfer: { files } })
 
-      const updatedDropzone = await flushPromises(dropzone)
+      const updatedDropzone = await flushPromises(component)
       const child = updatedDropzone.find(DummyChildComponent)
 
       expect(child).toHaveProp('isDragActive', true)
@@ -602,7 +658,7 @@ describe('Dropzone', () => {
       expect(dragActiveChild).toHaveProp('isDragAccept', true)
       expect(dragActiveChild).toHaveProp('isDragReject', false)
 
-      dropzone.simulate('dragLeave', { dataTransfer: { files } })
+      await dropzone.simulate('dragLeave', { dataTransfer: { files } })
       expect(dropzone.find(DragActiveComponent).children()).toHaveLength(0)
       expect(dropzone.find(ChildComponent)).toHaveProp('isDragAccept', false)
       expect(dropzone.find(ChildComponent)).toHaveProp('isDragReject', false)
@@ -765,13 +821,7 @@ describe('Dropzone', () => {
           accept="image/*"
         />
       )
-      const bogusImages = [
-        {
-          name: 'bogus.gif',
-          size: 1234,
-          type: 'application/x-moz-file'
-        }
-      ]
+      const bogusImages = [createFile('bogus.gif', 1234, 'application/x-moz-file')]
 
       await dropzone.simulate('drop', { dataTransfer: { files: bogusImages } })
       expect(onDrop).toHaveBeenCalledWith(bogusImages, [], expectedEvent)

src/utils/index.js

@@ -15,7 +15,11 @@ export function getDataTransferItems(event) {
     } else if (dt.items && dt.items.length) {
       // During the drag even the dataTransfer.files is null
       // but Chrome implements some drag store, which is accesible via dataTransfer.items
-      dataTransferItemsList = dt.items
+      // Map the items to File objects,
+      // and filter non-File items
+      // see https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/getAsFile
+      const files = Array.prototype.map.call(dt.items, item => item.getAsFile())
+      dataTransferItemsList = Array.prototype.filter.call(files, file => file !== null)
     }
   } else if (event.target && event.target.files) {
     dataTransferItemsList = event.target.files
@@ -39,6 +43,16 @@ export function allFilesAccepted(files, accept) {
   return files.every(file => fileAccepted(file, accept))
 }
 
+export function hasFiles(files) {
+  // Allow only files and retun the items as a list of File,
+  // see https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem for details
+  return (
+    Array.isArray(files) &&
+    files.length > 0 &&
+    Array.prototype.every.call(files, file => file instanceof File)
+  )
+}
+
 // allow the entire document to be a drag target
 export function onDocumentDragOver(evt) {
   evt.preventDefault()