From 65f255164594639797022f71e9779d1d7519214c Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 4 Aug 2022 16:21:23 +0200 Subject: [PATCH] @uppy/utils: modernize `getDroppedFiles` (#3534) `webkitGetAsEntry` is a non-standard/deprecated API, replacing it with `getAsFileSystemHandle` when available. This also work around a Chromium bug with symlinks. Fixes: https://github.com/transloadit/uppy/issues/3505. --- e2e/cypress/fixtures/images/cat-symbolic-link | 1 + .../fixtures/images/cat-symbolic-link.jpg | 1 + e2e/cypress/integration/dashboard-ui.spec.ts | 16 +++ .../@uppy/utils/src/getDroppedFiles/index.js | 13 ++- .../getFilesAndDirectoriesFromDirectory.js | 4 +- .../utils/webkitGetAsEntryApi/index.js | 99 ++++++++++--------- 6 files changed, 81 insertions(+), 53 deletions(-) create mode 120000 e2e/cypress/fixtures/images/cat-symbolic-link create mode 120000 e2e/cypress/fixtures/images/cat-symbolic-link.jpg diff --git a/e2e/cypress/fixtures/images/cat-symbolic-link b/e2e/cypress/fixtures/images/cat-symbolic-link new file mode 120000 index 0000000000..d9dd9f2118 --- /dev/null +++ b/e2e/cypress/fixtures/images/cat-symbolic-link @@ -0,0 +1 @@ +./cat.jpg \ No newline at end of file diff --git a/e2e/cypress/fixtures/images/cat-symbolic-link.jpg b/e2e/cypress/fixtures/images/cat-symbolic-link.jpg new file mode 120000 index 0000000000..d9dd9f2118 --- /dev/null +++ b/e2e/cypress/fixtures/images/cat-symbolic-link.jpg @@ -0,0 +1 @@ +./cat.jpg \ No newline at end of file diff --git a/e2e/cypress/integration/dashboard-ui.spec.ts b/e2e/cypress/integration/dashboard-ui.spec.ts index f68ec8b156..f322ead87f 100644 --- a/e2e/cypress/integration/dashboard-ui.spec.ts +++ b/e2e/cypress/integration/dashboard-ui.spec.ts @@ -2,6 +2,7 @@ describe('dashboard-ui', () => { beforeEach(() => { cy.visit('/dashboard-ui') cy.get('.uppy-Dashboard-input:first').as('file-input') + cy.get('.uppy-Dashboard-AddFiles').as('drop-target') }) it('should not throw when calling uppy.close()', () => { @@ -18,4 +19,19 @@ describe('dashboard-ui', () => { .should('have.length', 2) .each((element) => expect(element).attr('src').to.include('blob:')) }) + + it('should support drag&drop', () => { + cy.get('@drop-target').selectFile([ + 'cypress/fixtures/images/cat.jpg', + 'cypress/fixtures/images/cat-symbolic-link', + 'cypress/fixtures/images/cat-symbolic-link.jpg', + 'cypress/fixtures/images/traffic.jpg', + ], { action: 'drag-drop' }) + + cy.get('.uppy-Dashboard-Item') + .should('have.length', 4) + cy.get('.uppy-Dashboard-Item-previewImg') + .should('have.length', 3) + .each((element) => expect(element).attr('src').to.include('blob:')) + }) }) diff --git a/packages/@uppy/utils/src/getDroppedFiles/index.js b/packages/@uppy/utils/src/getDroppedFiles/index.js index 0db81b8301..8758db6ab2 100644 --- a/packages/@uppy/utils/src/getDroppedFiles/index.js +++ b/packages/@uppy/utils/src/getDroppedFiles/index.js @@ -15,11 +15,16 @@ import fallbackApi from './utils/fallbackApi.js' * * @returns {Promise} - Array */ -export default function getDroppedFiles (dataTransfer, { logDropError = () => {} } = {}) { +export default async function getDroppedFiles (dataTransfer, { logDropError = () => {} } = {}) { // Get all files from all subdirs. Works (at least) in Chrome, Mozilla, and Safari - if (dataTransfer.items?.[0] && 'webkitGetAsEntry' in dataTransfer.items[0]) { - return webkitGetAsEntryApi(dataTransfer, logDropError) + try { + const accumulator = [] + for await (const file of webkitGetAsEntryApi(dataTransfer, logDropError)) { + accumulator.push(file) + } + return accumulator // Otherwise just return all first-order files + } catch { + return fallbackApi(dataTransfer) } - return fallbackApi(dataTransfer) } diff --git a/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/getFilesAndDirectoriesFromDirectory.js b/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/getFilesAndDirectoriesFromDirectory.js index 42bb15aa61..65cbc174b9 100644 --- a/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/getFilesAndDirectoriesFromDirectory.js +++ b/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/getFilesAndDirectoriesFromDirectory.js @@ -13,9 +13,9 @@ export default function getFilesAndDirectoriesFromDirectory (directoryReader, ol // According to the FileSystem API spec, getFilesAndDirectoriesFromDirectory() // must be called until it calls the onSuccess with an empty array. if (entries.length) { - setTimeout(() => { + queueMicrotask(() => { getFilesAndDirectoriesFromDirectory(directoryReader, newEntries, logDropError, { onSuccess }) - }, 0) + }) // Done iterating this particular directory } else { onSuccess(newEntries) diff --git a/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js b/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js index ab97e6ec9a..8db7fb029e 100644 --- a/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js +++ b/packages/@uppy/utils/src/getDroppedFiles/utils/webkitGetAsEntryApi/index.js @@ -1,56 +1,61 @@ import getRelativePath from './getRelativePath.js' import getFilesAndDirectoriesFromDirectory from './getFilesAndDirectoriesFromDirectory.js' -import toArray from '../../../toArray.js' -export default function webkitGetAsEntryApi (dataTransfer, logDropError) { - const files = [] - - const rootPromises = [] - - /** - * Returns a resolved promise, when :files array is enhanced - * - * @param {(FileSystemFileEntry|FileSystemDirectoryEntry)} entry - * @returns {Promise} - empty promise that resolves when :files is enhanced with a file - */ - const createPromiseToAddFileOrParseDirectory = (entry) => new Promise((resolve) => { - // This is a base call - if (entry.isFile) { - // Creates a new File object which can be used to read the file. - entry.file( - (file) => { - // eslint-disable-next-line no-param-reassign - file.relativePath = getRelativePath(entry) - files.push(file) - resolve() - }, - // Make sure we resolve on error anyway, it's fine if only one file couldn't be read! - (error) => { - logDropError(error) - resolve() - }, - ) - // This is a recursive call - } else if (entry.isDirectory) { +/** + * Interop between deprecated webkitGetAsEntry and standard getAsFileSystemHandle. + */ +function getAsFileSystemHandleFromEntry (entry, logDropError) { + if (entry == null) return entry + return { + // eslint-disable-next-line no-nested-ternary + kind: entry.isFile ? 'file' : entry.isDirectory ? 'directory' : undefined, + getFile () { + return new Promise((resolve, reject) => entry.file(resolve, reject)) + }, + async* values () { + // If the file is a directory. const directoryReader = entry.createReader() - getFilesAndDirectoriesFromDirectory(directoryReader, [], logDropError, { - onSuccess: (entries) => resolve(Promise.all( - entries.map(createPromiseToAddFileOrParseDirectory), - )), + const entries = await new Promise(resolve => { + getFilesAndDirectoriesFromDirectory(directoryReader, [], logDropError, { + onSuccess: (dirEntries) => resolve(dirEntries.map(file => getAsFileSystemHandleFromEntry(file, logDropError))), + }) }) - } - }) + yield* entries + }, + } +} +async function* createPromiseToAddFileOrParseDirectory (entry) { // For each dropped item, - make sure it's a file/directory, and start deepening in! - toArray(dataTransfer.items) - .forEach((item) => { - const entry = item.webkitGetAsEntry() - // :entry can be null when we drop the url e.g. - if (entry) { - rootPromises.push(createPromiseToAddFileOrParseDirectory(entry)) - } - }) + if (entry.kind === 'file') { + const file = await entry.getFile() + if (file !== null) { + file.relativePath = getRelativePath(entry) + yield file + } + } else if (entry.kind === 'directory') { + for await (const handle of entry.values()) { + yield* createPromiseToAddFileOrParseDirectory(handle) + } + } +} - return Promise.all(rootPromises) - .then(() => files) +export default async function* getFilesFromDataTransfer (dataTransfer, logDropError) { + for (const item of dataTransfer.items) { + const lastResortFile = item.getAsFile() // Chromium bug, see https://github.com/transloadit/uppy/issues/3505. + const entry = await item.getAsFileSystemHandle?.() + ?? getAsFileSystemHandleFromEntry(item.webkitGetAsEntry(), logDropError) + // :entry can be null when we drop the url e.g. + if (entry != null) { + try { + yield* createPromiseToAddFileOrParseDirectory(entry, logDropError) + } catch (err) { + if (lastResortFile) { + yield lastResortFile + } else { + logDropError(err) + } + } + } + } }