upchunk icon indicating copy to clipboard operation
upchunk copied to clipboard

'progress' event is triggered right after 'success' event

Open dushajni opened this issue 3 years ago • 5 comments

As I wrote in the title, I noticed that progress event is triggered after upload is completed. In that way I have wrong info that upload is maybe not finished.

I consol.logged output and made this screenshot: SCREENSHOT

dushajni avatar Mar 24 '21 11:03 dushajni

Thanks for the report! We should be cancelling all event handlers after success, but could you tell me more about what this is causing in practice?

mmcc avatar Mar 31 '21 08:03 mmcc

Thanks for the report! We should be cancelling all event handlers after success, but could you tell me more about what this is causing in practice?

Sure. I've implemented mux inside of react / redux, and I'm dispatching actions for reducer on each event handler. So if I dispatch action for "upload completed" and after that I fire "upload in progress" it will make a mess to my logic. Upload in progress !== upload completed, so when I'm checking if download is in progress to execute some code, this additional "in progress" event will mislead.

dushajni avatar Apr 07 '21 09:04 dushajni

Same for me. I use progress to show the percentage and nullify it on success. A new progress event set again the percentage showing the wrong ui

ramiel avatar Oct 28 '21 11:10 ramiel

Running into a similar issue. progress events continue to fire after I've paused the upload. Creating weird effects in my UI.

adibbz avatar Dec 07 '22 18:12 adibbz

Thanks for re-surfacing @adibbz -- also want to just double check with folks on this thread if it's still happening on the latest release that went out 9 days ago: https://github.com/muxinc/upchunk/releases/tag/v3.0.0

There was a significant refactoring to improve the internals of how files were consuming memory (#89), I don't think it touched anything with how events were fired, but would be good to confirm.

#89 was our highest priority fix and now that's merged I'm hoping that will free up some bandwidth for other fixes like this one.

dylanjha avatar Dec 08 '22 01:12 dylanjha