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

Hook and callback changes #359

Closed ilessiivi closed 2 years ago

ilessiivi commented 2 years ago
drzraf commented 2 years ago

Quoting the README:

filter-file(<FlowFile> file, event) : boolean The boolean return value decide whether this particular file must be processed or ignored

It's the only one checking the return value. This change implies that if multiple hooks are attached, then one true would keep the file whereas the previous behavior was that one false would skip the file.

It's clearly an edge-case, but I still think that & the boolean values makes more sense so that false wins. (@AidasK )

(That being said, the include vs includes is clearly an important bugfix! :))

AidasK commented 2 years ago

I am not sure about this one either. Do we really need to invert this place? Could you give some examples why you think it's a bad behavior?

ilessiivi commented 2 years ago

The filter logic in the squashed commit is now checking that none of the hooks return false (I missed that this was the original behavior - sorry), and I added a new test to check for it in the async code path.

The second warning is now removed, though it'll be needed in the async hook later if the sync/async code paths get combined.

drzraf commented 2 years ago

Amazing!