Add @uppy/remote-sources preset/plugin #3676

arturi · 2022-04-29T18:06:34Z

With the new preset / after:

import Uppy from '@uppy/core'
import Dashboard from '@uppy/dashboard'
import RemoteSources from '@uppy/remote-sources'
import Webcam from '@uppy/webcam'
import Audio from '@uppy/audio'
import ScreenCapture from '@uppy/screen-capture'
import ImageEditor from '@uppy/image-editor'
import Compressor from '@uppy/compressor'
import Transloadit from '@uppy/transloadit'

const uppy = new Uppy()
  .use(Webcam)
  .use(Audio)
  .use(ImageEditor)
  .use(ScreenCapture)
  .use(Compressor)
  .use(Dashboard)
  .use(RemoteSources, { companionUrl: COMPANION_URL })
  .use(Transloadit, { waitForEncoding: true })

Without the new preset / before:

import Uppy from '@uppy/core'
import Dashboard from '@uppy/dashboard'
import Instagram from '@uppy/instagram'
import Facebook from '@uppy/facebook'
import OneDrive from '@uppy/onedrive'
import Dropbox from '@uppy/dropbox'
import Box from '@uppy/box'
import GoogleDrive from '@uppy/google-drive'
import Unsplash from '@uppy/unsplash'
import Zoom from '@uppy/zoom'
import Url from '@uppy/url'
import Webcam from '@uppy/webcam'
import ScreenCapture from '@uppy/screen-capture'
import Transloadit from '@uppy/transloadit'
import ImageEditor from '@uppy/image-editor'
import Audio from '@uppy/audio'
import Compressor from '@uppy/compressor'

const uppy = new Uppy()
  .use(Webcam)
  .use(Audio)
  .use(ImageEditor)
  .use(ScreenCapture)
  .use(Compressor)
  .use(Dashboard)
  .use(GoogleDrive, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Instagram, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Dropbox, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Box, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Facebook, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(OneDrive, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Zoom, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Url, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Unsplash, { target: Dashboard, companionUrl: COMPANION_URL })
  .use(Transloadit, { waitForEncoding: true })

Murderlon

I know it's WIP but there still needs to be a way to pass options. I think (hope) that all remote plugins share the exact same options.

packages/@uppy/remote-sources/README.md

packages/@uppy/remote-sources/types/index.d.ts

arturi · 2022-05-02T10:45:29Z

I know it's WIP but there still needs to be a way to pass options. I think (hope) that all remote plugins share the exact same options.

As far as I know, the only option for remote sources is companionUrl, is there something I'm missing?

Murderlon · 2022-05-02T14:13:34Z

As far as I know, the only option for remote sources is companionUrl, is there something I'm missing?

All these options and the ones in other docs.

Co-authored-by: Merlijn Vos <merlijn@soverin.net>

…e-sources

.eslintrc.js

packages/@uppy/remote-sources/README.md

packages/@uppy/remote-sources/src/index.js

packages/uppy/index.mjs

…ore robust

.eslintrc.js

packages/@uppy/remote-sources/package.json

packages/@uppy/remote-sources/src/index.js

aduh95 · 2022-05-30T09:48:19Z

packages/@uppy/remote-sources/src/index.js

+    this.opts.sources.forEach((pluginName) => {
+      const plugin = this.uppy.getPlugin(pluginName)
+      this.uppy.removePlugin(plugin)
+    })


Shouldn't be worried that this.opts.sources can be modified by the user and the uninstall would not work correctly? Another (better?) way of handling it would be to have a private Set on the instance that caches all the plugins that were installed.

Suggested change

this.opts.sources.forEach((pluginName) => {

const plugin = this.uppy.getPlugin(pluginName)

this.uppy.removePlugin(plugin)

})

this.#installedPlugins.forEach(plugin => this.uppy.removePlugin(plugin))

this.#installedPlugins.clear()

Yeah, it sounds better, not sure if Set is required or just a private array. But I did think of this, if you modify options the correct way via uppy.getPlugin('RemoteSources').setOptions({ sources: ['Instagram'] }), the plugin will first uninstall everything, then update options and install back: https://github.com/transloadit/uppy/pull/3676/files#diff-3b90938b025125a702e11b69f66acdd0c6985a892f64f4a28b06f0dcdb24e873R51-R55

I think we should not assume that users will always use the correct way.

const sources = ['Instagram'] uppy.use(RemoteSources, { sources }) sources.pop() // The user mutates the array for some reason uppy.getPlugin('RemoteSources').uninstall() // The Instagram plugin is still installed

I think Set is more suited for our use case, but I suppose an array would also work.

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

aduh95 · 2022-05-30T12:26:56Z

e2e/clients/dashboard-tus/app.js

+  .use(Tus, {
+    endpoint: 'https://tusd.tusdemo.net/files',
+    onShouldRetry,
+    retryDelays: [2000, 3000, 4000],


That looks unrelated, can we put it in its own PR (or remove it)

aduh95 · 2022-05-30T12:27:13Z

e2e/cypress/integration/dashboard-tus.spec.ts

    cy.intercept(
-      { method: 'PATCH', pathname: '/files/*', times: 2 },
+      { method: 'PATCH', pathname: '/files/*', times: 1 },
      { statusCode: 429, body: {} },
    ).as('patch')

-    cy.wait('@patch')
    cy.wait('@patch')


…f test more robust" This reverts commit 76a760c.

arturi · 2022-06-01T17:01:14Z

@aduh95 please take another look, cause I’m not sure what I’m doing with Set() 🙏

packages/@uppy/remote-sources/src/index.js

aduh95 · 2022-06-01T19:57:34Z

packages/@uppy/remote-sources/src/index.js

+      const optsForRemoteSourcePlugin = { ...this.opts, sources: undefined }
+      const plugin = availablePlugins.find(p => p.name === pluginId)
+      this.uppy.use(plugin, optsForRemoteSourcePlugin)


We could have a more useful error message here (maybe it'd be worth having a unit test btw?)

Suggested change

const optsForRemoteSourcePlugin = { ...this.opts, sources: undefined }

const plugin = availablePlugins.find(p => p.name === pluginId)

this.uppy.use(plugin, optsForRemoteSourcePlugin)

const plugin = availablePlugins.find(p => p.name === pluginId)

if (plugin == null) {

const formatter = new Intl.ListFormat('en', { style: 'long', type: 'disjunction' });

throw new Error(`Invalid plugin name: "${pluginId}" is not one of ${formatter.format(availablePlugins)}.`)

}

this.uppy.use(plugin, { ...this.opts, sources: undefined })

aduh95 · 2022-06-07T05:54:57Z

private/dev/Dashboard.js

+      sources: ['Box', 'Dropbox', 'Webcam', 'Facebook', 'GoogleDrive', 'Instagram', 'OneDrive', 'Unsplash', 'Url'],
    })
    .use(Webcam, {


Is it expected that Webcam is both in RemoteSources and as standalone?
Also, optional nit, can we sort the list to be in alphabetical order.

packages/@uppy/remote-sources/src/index.js

private/dev/Dashboard.js

Murderlon · 2022-06-07T10:20:21Z

website/src/docs/remote-sources.md

+tagline: "Uppy plugin that includes all remote sources that Uppy+Companion offer, like Instagram, Google Drive, Dropox, Box, Unsplash, Url etc"
+---
+
+Remote Sources plugin makes it convinient to add all the available remote sources — Instagram, Google Drive, Unsplash, Url, etc — to Uppy Dashboard in one convinient package.


Suggested change

Remote Sources plugin makes it convinient to add all the available remote sources — Instagram, Google Drive, Unsplash, Url, etc — to Uppy Dashboard in one convinient package.

@uppy/remote-sources is a preset plugin to add all the available remote sources, such Instagram, Google Drive, Dropbox, and others to Uppy Dashboard in one package.

Murderlon · 2022-06-07T10:25:51Z

website/src/docs/remote-sources.md

+})
+```
+
+You can possible to control which plugins will be used and the order they appear in the Dashboard, see below for `sources` option. All Companion-related options, such as `companionUrl`, are passed to the underlining plugins.


I think this sentence is redundant (it also has quite a few typos) as it just says 'hey we also have these two options' but you can also see that below in more detail

Suggested change

You can possible to control which plugins will be used and the order they appear in the Dashboard, see below for `sources` option. All Companion-related options, such as `companionUrl`, are passed to the underlining plugins.

Murderlon · 2022-06-07T10:28:34Z

website/src/docs/remote-sources.md

+* Type: `string`
+* Default: `RemoteSources`


Could you align all of these with how I have been doing this in the new website? It would save me a rewrite.

The format is:

[one sentence explanation] ([type], Default: [default])

Example:

Render the Dashboard as a modal or inline (`Boolean`, default: `false`).

Yeah, I like your way, just did this for consistency with the current docs.

Co-authored-by: Merlijn Vos <merlijn@soverin.net>

arturi · 2022-06-07T14:01:36Z

All feedback addressed, I think this is ready to be merged.

packages/@uppy/remote-sources/src/index.test.js

| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 2.2.1 | @uppy/tus | 2.4.1 | | @uppy/aws-s3-multipart | 2.4.1 | @uppy/url | 2.2.0 | | @uppy/companion-client | 2.2.1 | @uppy/xhr-upload | 2.1.2 | | @uppy/core | 2.3.1 | @uppy/robodog | 2.8.0 | | @uppy/react | 2.2.2 | uppy | 2.12.0 | | @uppy/remote-sources | 0.1.0 | | | - @uppy/remote-sources: Add @uppy/remote-sources preset/plugin (Artur Paikin / #3676) - @uppy/react: Reset uppy instance when React component is unmounted (Tomasz Pęksa / #3814) - @uppy/aws-s3-multipart,@uppy/aws-s3,@uppy/tus: queue socket token requests for remote files (Merlijn Vos / #3797) - @uppy/xhr-upload: replace `ev.target.status` with `xhr.status` (Wes Sankey / #3782) - @uppy/core: fix `TypeError` when file was deleted (Antoine du Hamel / #3811) - @uppy/robodog: fix linter warnings (Antoine du Hamel / #3808) - meta: fix GHA workflow for prereleases (Antoine du Hamel) - @uppy/aws-s3-multipart: allow `companionHeaders` to be modified with `setOptions` (Paulo Lemos Neto / #3770) - @uppy/url: enable passing optional meta data to `addFile` (Brad Edelman / #3788) - @uppy/url: fix `getFileNameFromUrl` (Brad Edelman / #3804) - @uppy/tus: make onShouldRetry type optional (Merlijn Vos / #3800) - doc: fix React examples (Antoine du Hamel / #3799) - meta: add GHA workflow for prereleases (Antoine du Hamel)

arturi added 2 commits April 29, 2022 19:03

Add @uppy/remote-sources preset/plugin

ac1f5df

yarn.lock

04dd8c7

Murderlon reviewed May 2, 2022

View reviewed changes

packages/@uppy/remote-sources/README.md Outdated Show resolved Hide resolved

packages/@uppy/remote-sources/types/index.d.ts Outdated Show resolved Hide resolved

arturi and others added 7 commits May 3, 2022 17:22

Update packages/@uppy/remote-sources/README.md

3fd0002

Co-authored-by: Merlijn Vos <merlijn@soverin.net>

Allow configuring sources, use Dashboard as default target

3af5fe9

Merge branch 'main' into remote-sources

c6cd1bb

Add remote-sources to the CDN bundle

54e51e7

Added docs, readme, types

d3eb0c0

Update yarn.lock

f65e456

Merge branch 'main' of https://github.com/transloadit/uppy into remot…

d2fe057

…e-sources

arturi marked this pull request as ready for review May 26, 2022 19:55

arturi requested review from Murderlon and aduh95 May 26, 2022 19:55

aduh95 reviewed May 26, 2022

View reviewed changes

.eslintrc.js Outdated Show resolved Hide resolved

packages/@uppy/remote-sources/README.md Outdated Show resolved Hide resolved

packages/@uppy/remote-sources/src/index.js Outdated Show resolved Hide resolved

packages/uppy/index.mjs Outdated Show resolved Hide resolved

arturi added 4 commits May 27, 2022 09:53

update readme, dev example, don't expose defaultOptions

c1bfc95

Merge branch 'main' into remote-sources

d249bea

Alphabetical order

fba2ffe

Increase retryDelays to hopefully make the exponential backoff test m…

76a760c

…ore robust

aduh95 reviewed May 30, 2022

View reviewed changes

arturi and others added 3 commits May 30, 2022 11:53

Update packages/@uppy/remote-sources/package.json

9bf0d67

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

Update packages/@uppy/remote-sources/src/index.js

f4ba2aa

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

Update packages/@uppy/remote-sources/src/index.js

8106fda

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

aduh95 reviewed May 30, 2022

View reviewed changes

arturi added 2 commits June 1, 2022 17:59

Store installedPlugins in a Set

18b1172

Revert "Increase retryDelays to hopefully make the exponential backof…

d686ebb

…f test more robust" This reverts commit 76a760c.

arturi requested a review from aduh95 June 1, 2022 17:00

aduh95 reviewed Jun 1, 2022

View reviewed changes

Require companionUrl and plugins stricly from the included, add tests

454ad85

aduh95 reviewed Jun 7, 2022

View reviewed changes

Murderlon reviewed Jun 7, 2022

View reviewed changes

arturi and others added 4 commits June 7, 2022 14:08

Update packages/@uppy/remote-sources/src/index.js

a455bf8

Co-authored-by: Merlijn Vos <merlijn@soverin.net>

Update private/dev/Dashboard.js

15e715d

Co-authored-by: Merlijn Vos <merlijn@soverin.net>

Update doc option type format

e57111e

Better intro from Merlijn

b498178

Murderlon approved these changes Jun 7, 2022

View reviewed changes

aduh95 reviewed Jun 7, 2022

View reviewed changes

packages/@uppy/remote-sources/src/index.test.js Outdated Show resolved Hide resolved

aduh95 reviewed Jun 7, 2022

View reviewed changes

packages/@uppy/remote-sources/src/index.test.js Outdated Show resolved Hide resolved

Apply suggestions from code review

fa53fbc

aduh95 merged commit 9a59711 into main Jun 7, 2022

aduh95 deleted the remote-sources branch June 7, 2022 16:56

github-actions bot mentioned this pull request Jun 7, 2022

Release: uppy@2.12.0 #3816

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add @uppy/remote-sources preset/plugin #3676

Add @uppy/remote-sources preset/plugin #3676

arturi commented Apr 29, 2022 •

edited

Murderlon left a comment

arturi commented May 2, 2022 •

edited

Murderlon commented May 2, 2022

aduh95 May 30, 2022

arturi May 30, 2022

aduh95 May 30, 2022

aduh95 May 30, 2022

aduh95 May 30, 2022

arturi commented Jun 1, 2022

aduh95 Jun 1, 2022

aduh95 Jun 7, 2022

Murderlon Jun 7, 2022

Murderlon Jun 7, 2022

Murderlon Jun 7, 2022

arturi Jun 7, 2022

arturi commented Jun 7, 2022

	Remote Sources plugin makes it convinient to add all the available remote sources — Instagram, Google Drive, Unsplash, Url, etc — to Uppy Dashboard in one convinient package.
	@uppy/remote-sources is a preset plugin to add all the available remote sources, such Instagram, Google Drive, Dropbox, and others to Uppy Dashboard in one package.

Add @uppy/remote-sources preset/plugin #3676

Add @uppy/remote-sources preset/plugin #3676

Conversation

arturi commented Apr 29, 2022 • edited

Murderlon left a comment

Choose a reason for hiding this comment

arturi commented May 2, 2022 • edited

Murderlon commented May 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arturi commented Jun 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arturi commented Jun 7, 2022

arturi commented Apr 29, 2022 •

edited

arturi commented May 2, 2022 •

edited