Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support async function for initFileFn #296

Merged
merged 1 commit into from
Jul 30, 2020
Merged

Conversation

drzraf
Copy link
Collaborator

@drzraf drzraf commented May 29, 2020

Sample use-case:

new Flow({
    initFileFn: async function(flowObj) {
        const foo = await something();
    }
});

@AidasK
Copy link
Member

AidasK commented Jun 5, 2020

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

@drzraf
Copy link
Collaborator Author

drzraf commented Jun 5, 2020

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, #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
Copy link
Member

AidasK commented Jun 7, 2020

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
Copy link
Collaborator Author

drzraf commented Jul 30, 2020

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
Copy link
Member

AidasK commented Jul 30, 2020

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

@drzraf
Copy link
Collaborator Author

drzraf commented Dec 31, 2020

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
Copy link
Member

AidasK commented Jan 4, 2021

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

@AidasK
Copy link
Member

AidasK commented Jan 4, 2021

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

@evilaliv3
Copy link
Member

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
Copy link
Member

AidasK commented Jan 4, 2021

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.

drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 13, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 13, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 14, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 21, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 26, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jan 27, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Jun 22, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf pushed a commit to drzraf/flow.js that referenced this pull request Aug 6, 2021
This is a follow-up (and proper implementation) of flowjs#296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file hit back events firing.
(flowjs#319) in general and `fileAdded`
in particular (flowjs#271) and the current
confusión between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)
drzraf added a commit that referenced this pull request Aug 9, 2021
## Ability to use asynchronous initFileFn.
This is a follow-up (and proper implementation) of #296 (reverted in e867d54)

This could be use to initialize a stream, a reader, fetch a remote resource, ... during the FlowFile initialization.
`asyncAddFile` and `asyncAddFiles` are introduced which support this mechanism.
These function return promises of one (or multiple) FlowFile(s).

To implement this:
- An AsyncFlowFile class extending a FlowFile is created, overriding the bootstrap part
- In order to keep code-duplication low, a filterFileList generator yield files
  in way common to async and non-async addFiles() functions.

The possibly async' nature of initializing file affects events firing.
(#319) in general and `fileAdded`
in particular (#271) and the current
confusion between hooks altering bootstraping and simple events.

- fileAdded is now assumed an event. If we detect a return value, a warning is sent.
- preFilterFile is introduced and allow filtering the raw file before bootstrap()
- postFilterFile is introduced and allow filtering the FlowFile after a (possibly async) bootstrap()
- filesAdded is preserved

About parameters:
- `initFileFn` is only used during bootstrap
- If such a helper exists, it's tightly related to addFile* functions.
As such, it was deemed adequate to provide it as a parameter to `*addFile*()`
 (but `opts.initFileFn` is still supported)

An `Eventizer` class is added where all the necessary code exist.
- Hooks alter code processing and could be `synchronous` or not.
- Events are CustomEvent dispatched asynchronously for informative purpose.

A warning is triggered if an obsolete event name is used

## Other changes
* uploadNextChunk: use transpilation instead of relying upon the each() helper
* FlowFile: don't bootstrap within the constructor. This must be done manually now
* README/CHANGELOG
* Also trigger async hooks in sync-mode (but don't wait for them to return)

# tests:
- switch uploadSpec to createFakeServer
- move some useful helpers related to final file consistency
- bugfix. xhr_server.restore() is not enough. Recreating the object fix some failures under particular tests sequences
- added new tests for the async initFileFn and related asynAddFile(s) functions
- modified tests to take into account changes related to hooks & events

* builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants