flowjs / flow.js

A JavaScript library providing multiple simultaneous, stable, fault-tolerant and resumable/restartable file uploads via the HTML5 File API.
Other
2.96k stars 346 forks source link

support async function for initFileFn #296

Closed drzraf closed 4 years ago

drzraf commented 4 years ago

Sample use-case:

new Flow({
    initFileFn: async function(flowObj) {
        const foo = await something();
    }
});
AidasK commented 4 years ago

Such an intresting way of supporting promises. Though it would be nice that every function could support that.

drzraf commented 4 years ago

It's most necessary for initFileFn because flow.js caller code is not expecting a callback making it inherently synchronous.

I believe that for readFileFn which already relies on a callback it may be easier to find async' workaround (I've yet to test this). I'm currently relying on a patched flow.js for readFileFn too, but I must admit this readFileFn > readFinished callback(this.send()) > this.send() part of the code seems very convoluted to me. (Even more since I'm reading a stream of unknown length, https://github.com/flowjs/flow.js/issues/42)

Since the code already depends on grunt and transpilation and this part is a defacto promise situation, it may benefit from making it more Promise-based using modern async/await keywords.

AidasK commented 4 years ago

Yes, that would be great to refactor every callback to a promise based one. If you want, you can create a pull request for that and we will release it as a major version.

drzraf commented 4 years ago

But aside from a refactor, could this one PR make it?

I know it's part of more global problem which is about flow.js handling streams (in particular ReadableStream) and this is just one small step and another one (#304 could make workarounds like #300 unnecessary)

But a more general refactor would also see flow.js working on a lower level (blob, getReader & streams) instead of the DOM-specific concepts like drop-zone. (I've yet to find how to initialize flowObj from a home-made stream, eg: a download, instead of a regular <file> element)

AidasK commented 4 years ago

Sure, this one looks safe. Thanks for all your effort

drzraf commented 3 years ago

With the transpilation now in place, native async functions may be transformed into generators. As a consequence the 'then' in ret tweak is likely to not work anymore. (And no test were provided at that time that would confirm this)

AidasK commented 3 years ago

We should just drop this test. We should not support callbacks without a promise

AidasK commented 3 years ago

Let's break some things, v3 should be cleaner, not backward compatible

evilaliv3 commented 3 years ago

Let's break some things, v3 should be cleaner, not backward compatible

I agree. What would you like to accomplish in v3 @AidasK ? As soon that there is a prototype almost final i may try to use it within @globaleaks before stable release so to see if it works with a an existing real project.

AidasK commented 3 years ago

My only request for v3 version is to migrate all the state functions to promise based ones. Though @drzraf did much more and can comment on that.