소스 검색

@uppy/provider-views: fix race condition when adding folders (#4384)

* fix race condtion when adding files

don't call addFolder from listAllFiles
because that would call addFile before all folders were loaded

also remove unused selectedFolders state

* fix also the case of adding multiple folders

* fix todo: remove SharedHandler

* remove loaderWrapper

* fix logic

* fix the last race condition

* fix broken duplicate file check

* fix logic

* prettiyfy loop

* add user feedback

so they know that something is happening

* run corepack yarn run build:locale-pack

* Revert "run corepack yarn run build:locale-pack"

This reverts commit 85548ce2fc3688972d5145f8812f7feb45fd64af.

* Revert "add user feedback"

This reverts commit 4060019c359c529415118ca073c31b373e737f90.

* use async generator instead of p-map

* re-fix race-condition

* remove providerFileToId

as suggested by @arturi

* use addFiles instead of addFile

* rename function

* use provider-supplied file ID

instead of generating an ID, for providers that we whitelsit
this allows adding the same time many times (with a different path)

* call core directly

* improve dev dashboard

* disable experimental getAsFileSystemHandle

it seems to be causing problems when dropping folders with subfolders in newest chrome
e.g a folder with 50 files and a subfolder which in turn has another 50 files

also refactor and document the code more

* Update packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

* mov eto utils

---------

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Mikael Finstad 2 년 전
부모
커밋
3dd1e5e68c

+ 4 - 7
packages/@uppy/core/src/Uppy.js

@@ -8,7 +8,7 @@ import throttle from 'lodash.throttle'
 import DefaultStore from '@uppy/store-default'
 import getFileType from '@uppy/utils/lib/getFileType'
 import getFileNameAndExtension from '@uppy/utils/lib/getFileNameAndExtension'
-import generateFileID from '@uppy/utils/lib/generateFileID'
+import { getSafeFileId } from '@uppy/utils/lib/generateFileID'
 import supportsUploadProgress from './supportsUploadProgress.js'
 import getFileName from './getFileName.js'
 import { justErrorsLogger, debugLogger } from './loggers.js'
@@ -458,12 +458,9 @@ class Uppy {
     const fileName = getFileName(fileType, fileDescriptor)
     const fileExtension = getFileNameAndExtension(fileName).extension
     const isRemote = Boolean(fileDescriptor.isRemote)
-    const fileID = generateFileID({
-      ...fileDescriptor,
-      type: fileType,
-    })
+    const id = getSafeFileId(fileDescriptor)
 
-    if (this.checkIfFileAlreadyExists(fileID)) {
+    if (this.checkIfFileAlreadyExists(id)) {
       const error = new RestrictionError(this.i18n('noDuplicates', { fileName }))
       this.#informAndEmit(error, fileDescriptor)
       throw error
@@ -478,7 +475,7 @@ class Uppy {
 
     let newFile = {
       source: fileDescriptor.source || '',
-      id: fileID,
+      id,
       name: fileName,
       extension: fileExtension || '',
       meta: {

+ 105 - 116
packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx

@@ -1,5 +1,7 @@
 import { h } from 'preact'
 
+import { getSafeFileId } from '@uppy/utils/lib/generateFileID'
+
 import AuthView from './AuthView.jsx'
 import Header from './Header.jsx'
 import Browser from '../Browser.jsx'
@@ -59,7 +61,6 @@ export default class ProviderView extends View {
     this.logout = this.logout.bind(this)
     this.handleAuth = this.handleAuth.bind(this)
     this.handleScroll = this.handleScroll.bind(this)
-    this.listAllFiles = this.listAllFiles.bind(this)
     this.donePicking = this.donePicking.bind(this)
 
     // Visual
@@ -101,29 +102,31 @@ export default class ProviderView extends View {
    * @param  {string} id Folder id
    * @returns {Promise}   Folders/files in folder
    */
-  getFolder (id, name) {
-    return this.sharedHandler.loaderWrapper(
-      this.provider.list(id),
-      (res) => {
-        const folders = []
-        const files = []
-        let updatedDirectories
-
-        const state = this.plugin.getPluginState()
-        const index = state.directories.findIndex((dir) => id === dir.id)
-
-        if (index !== -1) {
-          updatedDirectories = state.directories.slice(0, index + 1)
-        } else {
-          updatedDirectories = state.directories.concat([{ id, title: name }])
-        }
+  async getFolder (id, name) {
+    this.setLoading(true)
+    try {
+      const res = await this.provider.list(id)
+      const folders = []
+      const files = []
+      let updatedDirectories
+
+      const state = this.plugin.getPluginState()
+      const index = state.directories.findIndex((dir) => id === dir.id)
+
+      if (index !== -1) {
+        updatedDirectories = state.directories.slice(0, index + 1)
+      } else {
+        updatedDirectories = state.directories.concat([{ id, title: name }])
+      }
 
-        this.username = res.username || this.username
-        this.#updateFilesAndFolders(res, files, folders)
-        this.plugin.setPluginState({ directories: updatedDirectories, filterInput: '' })
-      },
-      this.handleError,
-    )
+      this.username = res.username || this.username
+      this.#updateFilesAndFolders(res, files, folders)
+      this.plugin.setPluginState({ directories: updatedDirectories, filterInput: '' })
+    } catch (err) {
+      this.handleError(err)
+    } finally {
+      this.setLoading(false)
+    }
   }
 
   /**
@@ -171,74 +174,6 @@ export default class ProviderView extends View {
     this.plugin.setPluginState({ filterInput: '' })
   }
 
-  /**
-   * Adds all files found inside of specified folder.
-   *
-   * Uses separated state while folder contents are being fetched and
-   * mantains list of selected folders, which are separated from files.
-   */
-  addFolder (folder) {
-    const folderId = this.providerFileToId(folder)
-    const folders = { ...this.plugin.getPluginState().selectedFolders }
-
-    if (folderId in folders && folders[folderId].loading) {
-      return
-    }
-
-    folders[folderId] = { loading: true, files: [] }
-
-    this.plugin.setPluginState({ selectedFolders: { ...folders } })
-
-    // eslint-disable-next-line consistent-return
-    return this.listAllFiles(folder.requestPath).then((files) => {
-      let count = 0
-
-      // If the same folder is added again, we don't want to send
-      // X amount of duplicate file notifications, we want to say
-      // the folder was already added. This checks if all files are duplicate,
-      // if that's the case, we don't add the files.
-      files.forEach(file => {
-        const id = this.providerFileToId(file)
-        if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) {
-          count++
-        }
-      })
-
-      if (count > 0) {
-        files.forEach((file) => this.addFile(file))
-      }
-
-      const ids = files.map(this.providerFileToId)
-
-      folders[folderId] = {
-        loading: false,
-        files: ids,
-      }
-      this.plugin.setPluginState({ selectedFolders: folders, filterInput: '' })
-
-      let message
-
-      if (count === 0) {
-        message = this.plugin.uppy.i18n('folderAlreadyAdded', {
-          folder: folder.name,
-        })
-      } else if (files.length) {
-        message = this.plugin.uppy.i18n('folderAdded', {
-          smart_count: count, folder: folder.name,
-        })
-      } else {
-        message = this.plugin.uppy.i18n('emptyFolderAdded')
-      }
-
-      this.plugin.uppy.info(message)
-    }).catch((e) => {
-      const selectedFolders = { ...this.plugin.getPluginState().selectedFolders }
-      delete selectedFolders[folderId]
-      this.plugin.setPluginState({ selectedFolders })
-      this.handleError(e)
-    })
-  }
-
   async handleAuth () {
     await this.provider.ensurePreAuth()
 
@@ -300,35 +235,91 @@ export default class ProviderView extends View {
     }
   }
 
-  async listAllFiles (path, files = null) {
-    files = files || [] // eslint-disable-line no-param-reassign
-    const res = await this.provider.list(path)
-    res.items.forEach((item) => {
-      if (!item.isFolder) {
-        files.push(item)
-      } else {
-        this.addFolder(item)
+  async* recursivelyListAllFiles (path) {
+    let curPath = path
+
+    // need to repeat the list call until there are no more pages
+    while (curPath) {
+      const res = await this.provider.list(curPath)
+
+      for (const item of res.items) {
+        if (item.isFolder) {
+          // recursively call self for folder
+          yield* this.recursivelyListAllFiles(item.requestPath)
+        } else {
+          yield item
+        }
       }
-    })
-    const moreFiles = res.nextPagePath
-    if (moreFiles) {
-      return this.listAllFiles(moreFiles, files)
+
+      curPath = res.nextPagePath
     }
-    return files
   }
 
-  donePicking () {
-    const { currentSelection } = this.plugin.getPluginState()
-    const promises = currentSelection.map((file) => {
-      if (file.isFolder) {
-        return this.addFolder(file)
+  async donePicking () {
+    this.setLoading(true)
+    try {
+      const { currentSelection } = this.plugin.getPluginState()
+
+      const messages = []
+      const newFiles = []
+
+      // eslint-disable-next-line no-unreachable-loop
+      for (const file of currentSelection) {
+        if (file.isFolder) {
+          const { requestPath, name } = file
+          let isEmpty = true
+          let numNewFiles = 0
+
+          for await (const fileInFolder of this.recursivelyListAllFiles(requestPath)) {
+            const tagFile = this.getTagFile(fileInFolder)
+            const id = getSafeFileId(tagFile)
+            // If the same folder is added again, we don't want to send
+            // X amount of duplicate file notifications, we want to say
+            // the folder was already added. This checks if all files are duplicate,
+            // if that's the case, we don't add the files.
+            if (!this.plugin.uppy.checkIfFileAlreadyExists(id)) {
+              newFiles.push(fileInFolder)
+              numNewFiles++
+            }
+            isEmpty = false
+          }
+
+          let message
+          if (isEmpty) {
+            message = this.plugin.uppy.i18n('emptyFolderAdded')
+          } else if (numNewFiles === 0) {
+            message = this.plugin.uppy.i18n('folderAlreadyAdded', {
+              folder: name,
+            })
+          } else {
+            message = this.plugin.uppy.i18n('folderAdded', {
+              smart_count: numNewFiles, folder: name,
+            })
+          }
+
+          messages.push(message)
+        } else {
+          newFiles.push(file)
+        }
       }
-      return this.addFile(file)
-    })
 
-    this.sharedHandler.loaderWrapper(Promise.all(promises), () => {
+      // Note: this.plugin.uppy.addFiles must be only run once we are done fetching all files,
+      // because it will cause the loading screen to disappear,
+      // and that will allow the user to start the upload, so we need to make sure we have
+      // finished all async operations before we add any file
+      // see https://github.com/transloadit/uppy/pull/4384
+      this.plugin.uppy.log('Adding remote provider files')
+      this.plugin.uppy.addFiles(newFiles.map((file) => this.getTagFile(file)))
+
+      this.plugin.setPluginState({ filterInput: '' })
+      messages.forEach(message => this.plugin.uppy.info(message))
+
       this.clearSelection()
-    }, () => {})
+    } catch (err) {
+      this.handleError(err)
+    } finally {
+      this.setLoading(false)
+    }
   }
 
   render (state, viewOptions = {}) {
@@ -341,7 +332,7 @@ export default class ProviderView extends View {
 
     const targetViewOptions = { ...this.opts, ...viewOptions }
     const { files, folders, filterInput, loading, currentSelection } = this.plugin.getPluginState()
-    const { isChecked, toggleCheckbox, recordShiftKeyPress, filterItems } = this.sharedHandler
+    const { isChecked, toggleCheckbox, recordShiftKeyPress, filterItems } = this
     const hasInput = filterInput !== ''
     const headerProps = {
       showBreadcrumbs: targetViewOptions.showBreadcrumbs,
@@ -364,7 +355,6 @@ export default class ProviderView extends View {
       username: this.username,
       getNextFolder: this.getNextFolder,
       getFolder: this.getFolder,
-      filterItems: this.sharedHandler.filterItems,
 
       // For SearchFilterInput component
       showSearchFilter: targetViewOptions.showFilter,
@@ -378,7 +368,6 @@ export default class ProviderView extends View {
       noResultsLabel: i18n('noFilesFound'),
       logout: this.logout,
       handleScroll: this.handleScroll,
-      listAllFiles: this.listAllFiles,
       done: this.donePicking,
       cancel: this.cancelPicking,
       headerComponent: Header(headerProps),

+ 15 - 16
packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx

@@ -36,7 +36,6 @@ export default class SearchProviderView extends View {
     this.search = this.search.bind(this)
     this.clearSearch = this.clearSearch.bind(this)
     this.resetPluginState = this.resetPluginState.bind(this)
-    this.addFile = this.addFile.bind(this)
     this.handleScroll = this.handleScroll.bind(this)
     this.donePicking = this.donePicking.bind(this)
 
@@ -77,20 +76,22 @@ export default class SearchProviderView extends View {
     })
   }
 
-  search (query) {
+  async search (query) {
     const { searchTerm } = this.plugin.getPluginState()
     if (query && query === searchTerm) {
       // no need to search again as this is the same as the previous search
-      return undefined
+      return
     }
 
-    return this.sharedHandler.loaderWrapper(
-      this.provider.search(query),
-      (res) => {
-        this.#updateFilesAndInputMode(res, [])
-      },
-      this.handleError,
-    )
+    this.setLoading(true)
+    try {
+      const res = await this.provider.search(query)
+      this.#updateFilesAndInputMode(res, [])
+    } catch (err) {
+      this.handleError(err)
+    } finally {
+      this.setLoading(false)
+    }
   }
 
   clearSearch () {
@@ -122,11 +123,9 @@ export default class SearchProviderView extends View {
 
   donePicking () {
     const { currentSelection } = this.plugin.getPluginState()
-    const promises = currentSelection.map((file) => this.addFile(file))
-
-    this.sharedHandler.loaderWrapper(Promise.all(promises), () => {
-      this.resetPluginState()
-    }, () => {})
+    this.plugin.uppy.log('Adding remote search provider files')
+    this.plugin.uppy.addFiles(currentSelection.map((file) => this.getTagFile(file)))
+    this.resetPluginState()
   }
 
   render (state, viewOptions = {}) {
@@ -139,7 +138,7 @@ export default class SearchProviderView extends View {
 
     const targetViewOptions = { ...this.opts, ...viewOptions }
     const { files, folders, filterInput, loading, currentSelection } = this.plugin.getPluginState()
-    const { isChecked, toggleCheckbox, filterItems } = this.sharedHandler
+    const { isChecked, toggleCheckbox, filterItems } = this
     const hasInput = filterInput !== ''
 
     const browserProps = {

+ 0 - 100
packages/@uppy/provider-views/src/SharedHandler.js

@@ -1,100 +0,0 @@
-import remoteFileObjToLocal from '@uppy/utils/lib/remoteFileObjToLocal'
-
-export default class SharedHandler {
-  constructor (plugin) {
-    this.plugin = plugin
-    this.filterItems = this.filterItems.bind(this)
-    this.toggleCheckbox = this.toggleCheckbox.bind(this)
-    this.recordShiftKeyPress = this.recordShiftKeyPress.bind(this)
-    this.isChecked = this.isChecked.bind(this)
-    this.loaderWrapper = this.loaderWrapper.bind(this)
-  }
-
-  filterItems (items) {
-    const state = this.plugin.getPluginState()
-    if (!state.filterInput || state.filterInput === '') {
-      return items
-    }
-    return items.filter((folder) => {
-      return folder.name.toLowerCase().indexOf(state.filterInput.toLowerCase()) !== -1
-    })
-  }
-
-  recordShiftKeyPress (e) {
-    this.isShiftKeyPressed = e.shiftKey
-  }
-
-  /**
-   * Toggles file/folder checkbox to on/off state while updating files list.
-   *
-   * Note that some extra complexity comes from supporting shift+click to
-   * toggle multiple checkboxes at once, which is done by getting all files
-   * in between last checked file and current one.
-   */
-  toggleCheckbox (e, file) {
-    e.stopPropagation()
-    e.preventDefault()
-    e.currentTarget.focus()
-    const { folders, files } = this.plugin.getPluginState()
-    const items = this.filterItems(folders.concat(files))
-    // Shift-clicking selects a single consecutive list of items
-    // starting at the previous click and deselects everything else.
-    if (this.lastCheckbox && this.isShiftKeyPressed) {
-      const prevIndex = items.indexOf(this.lastCheckbox)
-      const currentIndex = items.indexOf(file)
-      const currentSelection = (prevIndex < currentIndex)
-        ? items.slice(prevIndex, currentIndex + 1)
-        : items.slice(currentIndex, prevIndex + 1)
-      const reducedCurrentSelection = []
-
-      // Check restrictions on each file in currentSelection,
-      // reduce it to only contain files that pass restrictions
-      for (const item of currentSelection) {
-        const { uppy } = this.plugin
-        const restrictionError = uppy.validateRestrictions(
-          remoteFileObjToLocal(item),
-          [...uppy.getFiles(), ...reducedCurrentSelection],
-        )
-
-        if (!restrictionError) {
-          reducedCurrentSelection.push(item)
-        } else {
-          uppy.info({ message: restrictionError.message }, 'error', uppy.opts.infoTimeout)
-        }
-      }
-      this.plugin.setPluginState({ currentSelection: reducedCurrentSelection })
-      return
-    }
-
-    this.lastCheckbox = file
-    const { currentSelection } = this.plugin.getPluginState()
-    if (this.isChecked(file)) {
-      this.plugin.setPluginState({
-        currentSelection: currentSelection.filter((item) => item.id !== file.id),
-      })
-    } else {
-      this.plugin.setPluginState({
-        currentSelection: currentSelection.concat([file]),
-      })
-    }
-  }
-
-  isChecked (file) {
-    const { currentSelection } = this.plugin.getPluginState()
-    // comparing id instead of the file object, because the reference to the object
-    // changes when we switch folders, and the file list is updated
-    return currentSelection.some((item) => item.id === file.id)
-  }
-
-  loaderWrapper (promise, then, catch_) {
-    promise
-      .then((result) => {
-        this.plugin.setPluginState({ loading: false })
-        then(result)
-      }).catch((err) => {
-        this.plugin.setPluginState({ loading: false })
-        catch_(err)
-      })
-    this.plugin.setPluginState({ loading: true })
-  }
-}

+ 83 - 26
packages/@uppy/provider-views/src/View.js

@@ -1,35 +1,20 @@
 import getFileType from '@uppy/utils/lib/getFileType'
 import isPreviewSupported from '@uppy/utils/lib/isPreviewSupported'
-import generateFileID from '@uppy/utils/lib/generateFileID'
-
-// TODO: now that we have a shared `View` class,
-// `SharedHandler` could be cleaned up and moved into here
-import SharedHandler from './SharedHandler.js'
+import remoteFileObjToLocal from '@uppy/utils/lib/remoteFileObjToLocal'
 
 export default class View {
   constructor (plugin, opts) {
     this.plugin = plugin
     this.provider = opts.provider
-    this.sharedHandler = new SharedHandler(plugin)
 
     this.isHandlingScroll = false
 
     this.preFirstRender = this.preFirstRender.bind(this)
     this.handleError = this.handleError.bind(this)
-    this.addFile = this.addFile.bind(this)
     this.clearSelection = this.clearSelection.bind(this)
     this.cancelPicking = this.cancelPicking.bind(this)
   }
 
-  // eslint-disable-next-line class-methods-use-this
-  providerFileToId (file) {
-    return generateFileID({
-      data: file,
-      name: file.name || file.id,
-      type: file.mimetype,
-    })
-  }
-
   preFirstRender () {
     this.plugin.setPluginState({ didFirstRender: true })
     this.plugin.onFirstRender()
@@ -70,9 +55,10 @@ export default class View {
     uppy.info({ message, details: error.toString() }, 'error', 5000)
   }
 
-  addFile (file) {
+  // todo document what is a "tagFile" or get rid of this concept
+  getTagFile (file) {
     const tagFile = {
-      id: this.providerFileToId(file),
+      id: file.id,
       source: this.plugin.id,
       data: file,
       name: file.name || file.id,
@@ -90,6 +76,7 @@ export default class View {
         },
         providerOptions: this.provider.opts,
         providerName: this.provider.name,
+        provider: this.provider.provider,
       },
     }
 
@@ -105,16 +92,86 @@ export default class View {
       if (file.author.url) tagFile.meta.authorUrl = file.author.url
     }
 
-    this.plugin.uppy.log('Adding remote file')
+    return tagFile
+  }
 
-    try {
-      this.plugin.uppy.addFile(tagFile)
-      return true
-    } catch (err) {
-      if (!err.isRestriction) {
-        this.plugin.uppy.log(err)
+  filterItems = (items) => {
+    const state = this.plugin.getPluginState()
+    if (!state.filterInput || state.filterInput === '') {
+      return items
+    }
+    return items.filter((folder) => {
+      return folder.name.toLowerCase().indexOf(state.filterInput.toLowerCase()) !== -1
+    })
+  }
+
+  recordShiftKeyPress = (e) => {
+    this.isShiftKeyPressed = e.shiftKey
+  }
+
+  /**
+   * Toggles file/folder checkbox to on/off state while updating files list.
+   *
+   * Note that some extra complexity comes from supporting shift+click to
+   * toggle multiple checkboxes at once, which is done by getting all files
+   * in between last checked file and current one.
+   */
+  toggleCheckbox = (e, file) => {
+    e.stopPropagation()
+    e.preventDefault()
+    e.currentTarget.focus()
+    const { folders, files } = this.plugin.getPluginState()
+    const items = this.filterItems(folders.concat(files))
+    // Shift-clicking selects a single consecutive list of items
+    // starting at the previous click and deselects everything else.
+    if (this.lastCheckbox && this.isShiftKeyPressed) {
+      const prevIndex = items.indexOf(this.lastCheckbox)
+      const currentIndex = items.indexOf(file)
+      const currentSelection = (prevIndex < currentIndex)
+        ? items.slice(prevIndex, currentIndex + 1)
+        : items.slice(currentIndex, prevIndex + 1)
+      const reducedCurrentSelection = []
+
+      // Check restrictions on each file in currentSelection,
+      // reduce it to only contain files that pass restrictions
+      for (const item of currentSelection) {
+        const { uppy } = this.plugin
+        const restrictionError = uppy.validateRestrictions(
+          remoteFileObjToLocal(item),
+          [...uppy.getFiles(), ...reducedCurrentSelection],
+        )
+
+        if (!restrictionError) {
+          reducedCurrentSelection.push(item)
+        } else {
+          uppy.info({ message: restrictionError.message }, 'error', uppy.opts.infoTimeout)
+        }
       }
-      return false
+      this.plugin.setPluginState({ currentSelection: reducedCurrentSelection })
+      return
+    }
+
+    this.lastCheckbox = file
+    const { currentSelection } = this.plugin.getPluginState()
+    if (this.isChecked(file)) {
+      this.plugin.setPluginState({
+        currentSelection: currentSelection.filter((item) => item.id !== file.id),
+      })
+    } else {
+      this.plugin.setPluginState({
+        currentSelection: currentSelection.concat([file]),
+      })
     }
   }
+
+  isChecked = (file) => {
+    const { currentSelection } = this.plugin.getPluginState()
+    // comparing id instead of the file object, because the reference to the object
+    // changes when we switch folders, and the file list is updated
+    return currentSelection.some((item) => item.id === file.id)
+  }
+
+  setLoading (loading) {
+    this.plugin.setPluginState({ loading })
+  }
 }

+ 22 - 0
packages/@uppy/utils/src/generateFileID.js

@@ -1,3 +1,5 @@
+import getFileType from './getFileType.js'
+
 function encodeCharacter (character) {
   return character.charCodeAt(0).toString(32)
 }
@@ -43,3 +45,23 @@ export default function generateFileID (file) {
 
   return id
 }
+
+// If the provider has a stable, unique ID, then we can use that to identify the file.
+// Then we don't have to generate our own ID, and we can add the same file many times if needed (different path)
+function hasFileStableId (file) {
+  if (!file.isRemote || !file.remote) return false
+  // These are the providers that it seems like have stable IDs for their files. The other's I haven't checked yet.
+  const stableIdProviders = new Set(['box', 'dropbox', 'drive', 'facebook', 'unsplash'])
+  return stableIdProviders.has(file.remote.provider)
+}
+
+export function getSafeFileId (file) {
+  if (hasFileStableId(file)) return file.id
+
+  const fileType = getFileType(file)
+
+  return generateFileID({
+    ...file,
+    type: fileType,
+  })
+}

+ 39 - 15
packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js

@@ -1,7 +1,8 @@
 import getFilesAndDirectoriesFromDirectory from './getFilesAndDirectoriesFromDirectory.js'
 
 /**
- * Interop between deprecated webkitGetAsEntry and standard getAsFileSystemHandle.
+ * Polyfill for the new (experimental) getAsFileSystemHandle API (using the popular webkitGetAsEntry behind the scenes)
+ * so that we can switch to the getAsFileSystemHandle API once it (hopefully) becomes standard
  */
 function getAsFileSystemHandleFromEntry (entry, logDropError) {
   if (entry == null) return entry
@@ -29,36 +30,59 @@ async function* createPromiseToAddFileOrParseDirectory (entry, relativePath, las
   // For each dropped item, - make sure it's a file/directory, and start deepening in!
   if (entry.kind === 'file') {
     const file = await entry.getFile()
-    if (file !== null) {
+    if (file != null) {
       file.relativePath = relativePath ? `${relativePath}/${entry.name}` : null
       yield file
     } else if (lastResortFile != null) yield lastResortFile
   } else if (entry.kind === 'directory') {
     for await (const handle of entry.values()) {
+      // Recurse on the directory, appending the dir name to the relative path
       yield* createPromiseToAddFileOrParseDirectory(handle, `${relativePath}/${entry.name}`)
     }
   } else if (lastResortFile != null) yield lastResortFile
 }
 
+/**
+ * Load all files from data transfer, and recursively read any directories.
+ * Note that IE is not supported for drag-drop, because IE doesn't support Data Transfers
+ *
+ * @param {DataTransfer} dataTransfer
+ * @param {*} logDropError on error
+ */
 export default async function* getFilesFromDataTransfer (dataTransfer, logDropError) {
-  const entries = await Promise.all(Array.from(dataTransfer.items, async item => {
-    const lastResortFile = item.getAsFile() // Chromium bug, see https://github.com/transloadit/uppy/issues/3505.
-    let entry
-    // IMPORTANT: Need to check isSecureContext *first* or else Chrome will crash when running in HTTP:
-    // https://github.com/transloadit/uppy/issues/4133
-    if (window.isSecureContext && item.getAsFileSystemHandle != null) entry = await item.getAsFileSystemHandle()
-    // fallback
-    entry ??= getAsFileSystemHandleFromEntry(item.webkitGetAsEntry(), logDropError)
+  // Retrieving the dropped items must happen synchronously
+  // otherwise only the first item gets treated and the other ones are garbage collected.
+  // https://github.com/transloadit/uppy/pull/3998
+  const fileSystemHandles = await Promise.all(Array.from(dataTransfer.items, async item => {
+    let fileSystemHandle
+
+    // TODO enable getAsFileSystemHandle API once we can get it working with subdirectories
+    // IMPORTANT: Need to check isSecureContext *before* calling getAsFileSystemHandle
+    // or else Chrome will crash when running in HTTP: https://github.com/transloadit/uppy/issues/4133
+    // if (window.isSecureContext && item.getAsFileSystemHandle != null) entry = await item.getAsFileSystemHandle()
 
-    return { lastResortFile, entry }
+    // `webkitGetAsEntry` exists in all popular browsers (including non-WebKit browsers),
+    // however it may be renamed to getAsEntry() in the future, so you should code defensively, looking for both.
+    // from https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/webkitGetAsEntry
+    const getAsEntry = () => (typeof item.getAsEntry === 'function' ? item.getAsEntry() : item.webkitGetAsEntry())
+    // eslint-disable-next-line prefer-const
+    fileSystemHandle ??= getAsFileSystemHandleFromEntry(getAsEntry(), logDropError)
+
+    return {
+      fileSystemHandle,
+      lastResortFile: item.getAsFile(), // can be used as a fallback in case other methods fail
+    }
   }))
 
-  for (const { lastResortFile, entry } of entries) {
-    // :entry can be null when we drop the url e.g.
-    if (entry != null) {
+  for (const { lastResortFile, fileSystemHandle } of fileSystemHandles) {
+    // fileSystemHandle and lastResortFile can be null when we drop an url.
+    if (fileSystemHandle != null) {
       try {
-        yield* createPromiseToAddFileOrParseDirectory(entry, '', lastResortFile)
+        yield* createPromiseToAddFileOrParseDirectory(fileSystemHandle, '', lastResortFile)
       } catch (err) {
+        // Example: If dropping a symbolic link, Chromium will throw:
+        // "DOMException: A requested file or directory could not be found at the time an operation was processed.",
+        // So we will use lastResortFile instead. See https://github.com/transloadit/uppy/issues/3505.
         if (lastResortFile != null) {
           yield lastResortFile
         } else {

+ 5 - 2
private/dev/Dashboard.js

@@ -55,6 +55,9 @@ async function assemblyOptions () {
 // Rest is implementation! Obviously edit as necessary...
 
 export default () => {
+  const restrictions = undefined
+  // const restrictions = { requiredMetaFields: ['caption'], maxNumberOfFiles: 3 }
+
   const uppyDashboard = new Uppy({
     logger: debugLogger,
     meta: {
@@ -62,7 +65,7 @@ export default () => {
       license: 'Creative Commons',
     },
     allowMultipleUploadBatches: false,
-    // restrictions: { requiredMetaFields: ['caption'] },
+    restrictions,
   })
     .use(Dashboard, {
       trigger: '#pick-files',
@@ -74,7 +77,7 @@ export default () => {
       ],
       showProgressDetails: true,
       proudlyDisplayPoweredByUppy: true,
-      note: '2 files, images and video only',
+      note: `${JSON.stringify(restrictions)}`,
     })
     // .use(GoogleDrive, { target: Dashboard, companionUrl: COMPANION_URL, companionAllowedHosts })
     // .use(Instagram, { target: Dashboard, companionUrl: COMPANION_URL, companionAllowedHosts })