From 3dd1e5e68ce3a252a02e08f55fd17e0cbfe46e06 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Tue, 4 Apr 2023 12:13:39 +0900 Subject: [PATCH] @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 * mov eto utils --------- Co-authored-by: Antoine du Hamel --- packages/@uppy/core/src/Uppy.js | 11 +- .../src/ProviderView/ProviderView.jsx | 221 +++++++++--------- .../SearchProviderView/SearchProviderView.jsx | 31 ++- .../@uppy/provider-views/src/SharedHandler.js | 100 -------- packages/@uppy/provider-views/src/View.js | 109 ++++++--- packages/@uppy/utils/src/generateFileID.js | 22 ++ .../utils/webkitGetAsEntryApi/index.js | 54 +++-- private/dev/Dashboard.js | 7 +- 8 files changed, 273 insertions(+), 282 deletions(-) delete mode 100644 packages/@uppy/provider-views/src/SharedHandler.js diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js index 852eee0597..ce45394923 100644 --- a/packages/@uppy/core/src/Uppy.js +++ b/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: { diff --git a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx index 8d97cd0931..3690cf6348 100644 --- a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx +++ b/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), diff --git a/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx b/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx index ad07679f65..c2bde793f7 100644 --- a/packages/@uppy/provider-views/src/SearchProviderView/SearchProviderView.jsx +++ b/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 = { diff --git a/packages/@uppy/provider-views/src/SharedHandler.js b/packages/@uppy/provider-views/src/SharedHandler.js deleted file mode 100644 index e7af174116..0000000000 --- a/packages/@uppy/provider-views/src/SharedHandler.js +++ /dev/null @@ -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 }) - } -} diff --git a/packages/@uppy/provider-views/src/View.js b/packages/@uppy/provider-views/src/View.js index 1ed2eae584..910a2ed8a3 100644 --- a/packages/@uppy/provider-views/src/View.js +++ b/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 }) + } } diff --git a/packages/@uppy/utils/src/generateFileID.js b/packages/@uppy/utils/src/generateFileID.js index 73a88fc4e9..bdb6367658 100644 --- a/packages/@uppy/utils/src/generateFileID.js +++ b/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, + }) +} diff --git a/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js b/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js index f89ba18c69..08ee4fd3b8 100644 --- a/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js +++ b/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 { diff --git a/private/dev/Dashboard.js b/private/dev/Dashboard.js index a1e5ad227f..0e98db614a 100644 --- a/private/dev/Dashboard.js +++ b/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 })