From d4a58d9f7a8cbcbb855e9ce11a753420c52bf620 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 13 Feb 2023 15:16:26 +0100 Subject: [PATCH 1/5] @uppy/transloadit: fix `assemblyOptions` option --- packages/@uppy/transloadit/src/AssemblyOptions.js | 10 +++++++--- packages/@uppy/transloadit/src/index.js | 2 +- private/dev/Dashboard.js | 6 +++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/@uppy/transloadit/src/AssemblyOptions.js b/packages/@uppy/transloadit/src/AssemblyOptions.js index 53bc44ecad..b557717b59 100644 --- a/packages/@uppy/transloadit/src/AssemblyOptions.js +++ b/packages/@uppy/transloadit/src/AssemblyOptions.js @@ -59,12 +59,16 @@ async function getAssemblyOptions (file, options) { } function getFields (file, assemblyOptions) { - if (Array.isArray(assemblyOptions.fields)) { + const { fields } = assemblyOptions + if (fields == null) { + return {} + } + if (Array.isArray(fields)) { return Object.fromEntries( - assemblyOptions.fields.map((fieldName) => [fieldName, file.meta[fieldName]]), + fields.map((fieldName) => [fieldName, file.meta[fieldName]]), ) } - return {} + return fields } /** diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index f5560eeede..a3aef96930 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -79,7 +79,7 @@ export default class Transloadit extends BasePlugin { this.i18nInit() - const hasCustomAssemblyOptions = this.opts.assemblyOptions !== defaultOptions.assemblyOptions + const hasCustomAssemblyOptions = this.opts.assemblyOptions !== defaultOptions.getAssemblyOptions if (this.opts.params) { validateParams(this.opts.params) } else if (!hasCustomAssemblyOptions) { diff --git a/private/dev/Dashboard.js b/private/dev/Dashboard.js index b04fe9580c..a1e5ad227f 100644 --- a/private/dev/Dashboard.js +++ b/private/dev/Dashboard.js @@ -41,7 +41,7 @@ console.log(import.meta.env) const RESTORE = false -async function getAssemblyOptions () { +async function assemblyOptions () { return generateSignatureIfSecret(TRANSLOADIT_SECRET, { auth: { key: TRANSLOADIT_KEY, @@ -124,7 +124,7 @@ export default () => { uppyDashboard.use(Transloadit, { service: TRANSLOADIT_SERVICE_URL, waitForEncoding: true, - getAssemblyOptions, + assemblyOptions, }) break case 'transloadit-s3': @@ -132,7 +132,7 @@ export default () => { uppyDashboard.use(Transloadit, { waitForEncoding: true, importFromUploadURLs: true, - getAssemblyOptions, + assemblyOptions, }) break case 'transloadit-xhr': From 605ef848584c51417e908063821795f62fe0b57d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 13 Feb 2023 15:38:20 +0100 Subject: [PATCH 2/5] fix default and fallback --- packages/@uppy/transloadit/src/index.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index a3aef96930..bf2e6832b7 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -65,7 +65,7 @@ export default class Transloadit extends BasePlugin { /** @deprecated use `assemblyOptions` instead */ params: null, /** @deprecated use `assemblyOptions` instead */ - fields: {}, + fields: null, /** @deprecated use `assemblyOptions` instead */ getAssemblyOptions: defaultGetAssemblyOptions, limit: 20, @@ -79,13 +79,12 @@ export default class Transloadit extends BasePlugin { this.i18nInit() - const hasCustomAssemblyOptions = this.opts.assemblyOptions !== defaultOptions.getAssemblyOptions - if (this.opts.params) { - validateParams(this.opts.params) - } else if (!hasCustomAssemblyOptions) { - // Throw the same error that we'd throw if the `params` returned from a - // `getAssemblyOptions()` function is null. - validateParams(null) + if (this.opts.assemblyOptions !== defaultOptions.getAssemblyOptions) { + const { params, fields, signature } = opts + if (params != null || fields != null || signature != null) { + validateParams(params) + this.opts.assemblyOptions = { params, signature, fields } + } } this.client = new Client({ From 2573c5cdcfe7bb2b6e86e2d29a097e0d1b4502a2 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 13 Feb 2023 15:55:09 +0100 Subject: [PATCH 3/5] simplify fallback --- packages/@uppy/transloadit/src/index.js | 30 +++++++++---------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index bf2e6832b7..348425ae38 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -5,20 +5,12 @@ import BasePlugin from '@uppy/core/lib/BasePlugin.js' import Tus from '@uppy/tus' import Assembly from './Assembly.js' import Client from './Client.js' -import AssemblyOptions, { validateParams } from './AssemblyOptions.js' +import AssemblyOptions from './AssemblyOptions.js' import AssemblyWatcher from './AssemblyWatcher.js' import locale from './locale.js' import packageJson from '../package.json' -function defaultGetAssemblyOptions (file, options) { - return { - params: options.params, - signature: options.signature, - fields: options.fields, - } -} - const sendErrorToConsole = originalErr => err => { const error = new ErrorWithCause('Failed to send error to the client', { cause: err }) // eslint-disable-next-line no-console @@ -67,26 +59,24 @@ export default class Transloadit extends BasePlugin { /** @deprecated use `assemblyOptions` instead */ fields: null, /** @deprecated use `assemblyOptions` instead */ - getAssemblyOptions: defaultGetAssemblyOptions, + getAssemblyOptions: null, limit: 20, retryDelays: [7_000, 10_000, 15_000, 20_000], } this.opts = { ...defaultOptions, ...opts } - // TODO: move this into `defaultOptions` once we remove the deprecated options - this.opts.assemblyOptions = opts.assemblyOptions ?? this.opts.getAssemblyOptions + + // TODO: remove this fallback in the next major + this.opts.assemblyOptions ??= this.opts.getAssemblyOptions ?? { + params: this.opts.params, + signature: this.opts.signature, + fields: this.opts.fields, + } + this.#rateLimitedQueue = new RateLimitedQueue(this.opts.limit) this.i18nInit() - if (this.opts.assemblyOptions !== defaultOptions.getAssemblyOptions) { - const { params, fields, signature } = opts - if (params != null || fields != null || signature != null) { - validateParams(params) - this.opts.assemblyOptions = { params, signature, fields } - } - } - this.client = new Client({ service: this.opts.service, client: this.#getClientVersion(), From cade8d219edc2ffeda1cfb9da946b33bd219d184 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 13 Feb 2023 16:07:16 +0100 Subject: [PATCH 4/5] update types --- packages/@uppy/transloadit/types/index.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/transloadit/types/index.d.ts b/packages/@uppy/transloadit/types/index.d.ts index 100db01fc5..e52fe60342 100644 --- a/packages/@uppy/transloadit/types/index.d.ts +++ b/packages/@uppy/transloadit/types/index.d.ts @@ -97,7 +97,7 @@ interface AssemblyParameters { interface AssemblyOptions { params?: AssemblyParameters - fields?: { [name: string]: number | string } + fields?: { [name: string]: number | string } | string[] // TODO (major): move signature into params.auth. signature?: string } @@ -143,7 +143,7 @@ export type TransloaditOptions = Options & /** @deprecated use `assemblyOptions` instead */ params?: AssemblyParameters /** @deprecated use `assemblyOptions` instead */ - fields?: { [name: string]: number | string } + fields?: { [name: string]: number | string } | string[] /** @deprecated use `assemblyOptions` instead */ signature?: string /** @deprecated use `assemblyOptions` instead */ From 7f72a97f698f100f1e5840197505abb6f724200f Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 13 Feb 2023 16:35:59 +0100 Subject: [PATCH 5/5] fix tests --- packages/@uppy/transloadit/src/index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/@uppy/transloadit/src/index.js b/packages/@uppy/transloadit/src/index.js index 348425ae38..57beda6cf6 100644 --- a/packages/@uppy/transloadit/src/index.js +++ b/packages/@uppy/transloadit/src/index.js @@ -5,7 +5,7 @@ import BasePlugin from '@uppy/core/lib/BasePlugin.js' import Tus from '@uppy/tus' import Assembly from './Assembly.js' import Client from './Client.js' -import AssemblyOptions from './AssemblyOptions.js' +import AssemblyOptions, { validateParams } from './AssemblyOptions.js' import AssemblyWatcher from './AssemblyWatcher.js' import locale from './locale.js' @@ -73,6 +73,11 @@ export default class Transloadit extends BasePlugin { fields: this.opts.fields, } + // TODO: remove this check in the next major (validating params when creating the assembly should be enough) + if (opts?.params != null && opts.getAssemblyOptions == null && opts.assemblyOptions == null) { + validateParams(this.opts.assemblyOptions.params) + } + this.#rateLimitedQueue = new RateLimitedQueue(this.opts.limit) this.i18nInit()