core: Remove .run().

[goto-bus-stop]

This removes the need for calling .run() manually.

Will wait for Artur to be back before going ahead with this tho ✌️

[arturi]

I like this 👍been meaning to remove .run() for some time. Quite a few people (myself included) forget to call .run() and that causes all sorts of side effects and results in issues where we’ll be like “could you check that you did .run()” at the end?

But I remember @tim-kos mentioned something that “it’s nice to be able to set things up, prepare everything, and then run at a particular moment when needed”. I think we don’t really do that, and if you really want things to run at a particular point, you could just place your .use() calls there?

An alternative solution would be to make Uppy less functional before .run() — move the plugin.install() calls there.

[goto-bus-stop]

Moving plugin.install() to run() would be nice … however right now the React components install() and uninstall() plugins lazily so I'm not sure how that would work :/

I agree having .run() end a chain of .use()s is nice tho. It makes it clear when the instance is considered "done".

(FWIW some other .use()-based libraries like Express also don't have an explicit .run() type thing)

[arturi]

however right now the React components install() and uninstall() plugins lazily

But currently we install them right away when they are used:

uppy/src/core/Core.js

Line 832 in 3769385

plugin.install()

Didn’t find install calls in React components here: https://github.com/transloadit/uppy/tree/master/src/react, did you mean something else?

(FWIW some other .use()-based libraries like Express also don't have an explicit .run() type thing)

Yeah, also noticed that, so I think that’s fine.

[goto-bus-stop]

Yes sorry 🙈 I meant that the React components .use() the plugins on mount, and rely on that immediately calling .install().
They also require that .use() can be called after .run().
We could change those to manually call .install() after doing .use(), or we could make .use() call install() only if .run() has already been called, or something else.

---

One cool thing about installing inside .run() is that order would no longer matter for target: Dashboard and plugins: ['Dropbox'] options!

[tim-kos]

I see a few PROs and CONs for removing .run:

PROs
P.1 Less code to write for the end user.

P.2 Less possible things to forget for the end user, which then would only create support tickets.

CONs
C.1 A clear initialisation phase with .use, where order does not matter.

C.2 Complete freedom to set things up inside without having to consider
that some parts might already have been .install()'ed or are run()ning already.

Without knowing super much about the inner workings of Uppy at this time, I have the feeling, that
.use() and .install() should more or less mean the same thing. The English words mean more or less the same thing
in the context of uploads I think.

What would speak against treating them as such? .use() sets up things, and nothing really visible happens before .run() is called.
So that for example, when somebody clicks "Browse files", it wouldn't even open the file browser in case .run() was not called.
That would be the most obvious thing. Then also maybe show a string in the console that if the page was fully loaded, but .run() then was not called within 2 seconds or whatever.

Also maybe we could make it super explicit in the docs and on a screen before posting a GitHub issue that people should check if .run() was called.
Sort of like we always demand an Assembly ID before people post a support ticket for Transloadit.

[goto-bus-stop]

So that for example, when somebody clicks "Browse files", it wouldn't even open the file browser in case .run() was not called. That would be the most obvious thing.

This is the current behaviour but it's quite confusing for users; often times .run() is forgotten and Uppy appears to start correctly, but nothing interactive works, which looks like a bug if you're not aware of .run().

Then also maybe show a string in the console that if the page was fully loaded, but .run() then was not called within 2 seconds or whatever.

I guess this would help. I would then prefer to also move the install() phase to run() tho.

[arturi]

This is the current behaviour

Not really, I think, because buttons and uploading works now without run, but progress is not shown.

I would then prefer to also move the install() phase to run() tho.

Yeah, then nothing loads and its much more apparent.

I’d be fine with both cases, personally I see that in Express and PostCSS, as Renee mentioned, .use() are meant to be used in order, and there’s no run, and that’s what people expect from the chained APIs like that — that they pipe files (or something else) through that chain, in order. Piping in not exactly what happens in Uppy, but nonetheless. So I’d be actually for just removing .run, like this PR does.

But since Tim (and I think Kevin @kvz) vote to keep it, and if we think using plugins in random order is a nice thing to have (is it? probably nice, but things work fine now?) then yes, we should make run “more necessary”.

[kvz]

(and I think Kevin @kvz) vote to keep it

FWIW I'm not in favor of keeping run. It's caused enough issues for me to want to get rid of it. I also think it's reasonable to have constraints regarding chaining order. If it so happens that we'll get even more issues from not having run—we can opt to bring it back :) That's my 2ct.

[arturi]

We’ve agreed to try it like this, and then revert if we miss .run().