Conversation
| super(uppy, opts) | ||
| this.id = this.opts.id || 'Url' | ||
| this.title = 'Link' | ||
| this.type = 'acquirer' |
There was a problem hiding this comment.
I think we should consider not hardcoding these plugin types.
We could have core.types = {ACQUIRER: 'acquirer', ...}
then in our plugin we do:
this.type = core.types.ACQUIRER
it might be easier to maintain that way. Especially with custom plugins created outside this project.
what do you think?
| this[method] = this[method].bind(this) | ||
| }) | ||
|
|
||
| this[this.id] = new Provider(uppy, { |
There was a problem hiding this comment.
yeah, I'm at crossroads with this one. Here are a few thoughts I have:
-
I think we should consider removing plugins/Provider from the plugins folder, because it is not a plugin. And it's the only defaulter there. Maybe we should move it to core? Or somewhere else?
-
Even if we do move plugins/Provider to a separate folder, I think its use doesn't apply to the
URLplugin. I don't consider the URL a Provider because, for GoogleDrive, Dropbox, etc, we have a definite source (aka Provider) they connect to (e.g drive.google.com for GoogleDrive). So the expected behavior can be managed for each Provider.
But for URLs, it would be connecting to different sources (depending on the URL provided), from which we can expect different behaviors (e.g some may require authorization headers etc). So it might be tricky to squeeze that into a Provider. Instead maybe we should have URL stand as a separate plugin and not considered it a Provider type.
- That said, I think we need a separate module that manages communication with uppy-server. So instead of just plugins/Provider which does it right now. We can have a folder structure like this:
ServerBridge(or a better name)
--- Provider
--- ApiCalls (or whatever)
where plugins like google drive can go on to use Provider, while plugins like URL, S3 can just use ApiCalls directly. And then going back to suggestion 1. we could move ServerBridge to Core or make it standalone.
What do you think? I hope I made sense 😄
There was a problem hiding this comment.
We discussed this in the call yesterday and decided to try the following: add generic API calls to provider/index.js so that we don’t have to repeat the boilerplate with fetch headers and so on. For now leave things as one this, provider, maybe think on a more generic name, like uppy-server-api or something.
Then maybe provider lives in uppy-core, but uppy-provider-views is a separate package.
| } | ||
|
|
||
| getMeta (url) { | ||
| return fetch(`${this.hostname}/url/meta`, { |
There was a problem hiding this comment.
based on my suggestion 2. and 3. above, we can have this fetch done in ApiCalls
src/plugins/Url/index.js
Outdated
|
|
||
| render (state) { | ||
| return <div class="uppy-Url"> | ||
| <input |
There was a problem hiding this comment.
I think if this view is not re-usable by any other plugin, then we can leave this as is, and no need to fuse this with Provider/view.
|
Cool! looks like you committed an outdated package-lock file though :) |
3 participants
Client side response to @ifedapoolarewaju work on uppy-server side transloadit/uppy-server@0a33b48.
With UI I think we could later make it the same as with other Providers and show a “select file” button below.
Code-wise, I hardcoded things in the Url plugin itself, didn’t separate the view and logic much there, @ifedapoolarewaju hoping we can find a way to make this more consistent with other Provider plugins.