Callbacks or event emitter
mbraak opened this issue · 8 comments
We now have two apis that handle callbacks: the callbacks
and the events like addUpload
.
Let's see if it is possible to use a single api.
- Possible solution is to use the
callbacks
option for all events. - The goal is to not break existing code.
- Possible step: see if the new events can be handled by the
callbacks
option.
Yes, it does appear to be some duplicated functionality here.
As far as I can tell, callbacks are only for Upload
, so they do not yet work for S3, placeholder, and reload of form. Of course more callbacks can be added, at least to S3, but the advantage of signal is that the same signal could be emitted by different mechanisms ( e.g. addUpload
by placeholder, S3, tus etc), whereas multiple callbacks would have to be defined for each of them. (I am likely wrong here).
On the other hand the event interface does not yet handle onProgress
, onCancel
, onError
etc, which I assume would be easy to add.
So
Possible solution is to use the callbacks option for all events.
I agree, but the events mechanism appears to be more powerful, and more modern.
The goal is to not break existing code.
If so decided, we could deprecate callbacks by removing its documentation but keeping the implementation for backward compatibility.
Possible step: see if the new events can be handled by the callbacks option.
Not possible without expanding callbacks, as far as I can tell.
Edit: Another thought, callbacks will be called in the same process/thread in which an event happens so they must be short. Events are executed asynchronously and can perform tasks that are more elaborative. In this regard events are flexible and will make the frontend more responsive. I would prefer events to callbacks.
Note that documentation on initUploadFields
misses option eventEmitter
.
options (optional)
callbacks: callbacks for things like upload progress and errors.
Thanks, I'll update the documentation.
I agree that we should use the event emitter mechanism and deprecate the callbacks option. There is one thing I like to change, though. I would like to remove the eventEmitter parameter, and do the following:
const fileForm = initUploadFields(...);
fileForm.on('addUpload', ...);
The only problem with this is that addUpload
events for the initial uploads are not handled. Because the events are already emitted after initUploadFields
is called.
The only problem with this is that addUpload events for the initial uploads are not handled. Because the events are already emitted after initUploadFields is called.
Then it will be difficult to implement our example, which has placeholders.
It must be possible to find a solution for the placeholders: perhaps an option onAddInitialUpload
. Another solution is to keep the eventEmitter
option, but also return a fileForm
object that emits events.
There is no activity on this issue. Closing it for now.