Skip to content
This repository has been archived by the owner on Feb 27, 2022. It is now read-only.

Keep the listeners from the original tasks when checkForExistingDownloads is called #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vinicz
Copy link

@vinicz vinicz commented Jul 10, 2020

First of all, this is definitely not a bug fix, just a suggestion :)

I know that the checkForExistingDownloads call is intended to only re-attach the lost downloads after the app is restarted somehow. But with this PR it could be easily extended to be able to control the currently running tasks without the need to keep all the task references in the application code. And still, you don't need to re-subscribe for the events after the tasks are resumed.

In our use case, we started a download for a lot of files in parallel and recently added the feature for pausing the whole process. A solution would have been to track all the tasks ourselves and control those, but we realized that the tasks already in a map on the native side and also in another map on the JS side. By adding these couple extra lines we were able to keep the original listeners on the tasks (so we could monitor the progress) and retrieve the running tasks anytime without keeping track of them in our code.

Let me know what do think.

@ptelad
Copy link
Contributor

ptelad commented Jul 19, 2020

Hi,
Thanks for the PR.
Is you use case on Android or iOS?
I can see a potential bug in iOS, so can you tell me if you got to test it there?

@vinicz
Copy link
Author

vinicz commented Jul 19, 2020

Hi @ptelad!
We tested both iOS and Android on multiple devices and didn't experience any issue so far. We already pushed out the update to production more than a week ago and we received no complaints from the users yet. But if you could describe the potential bug we would be happy to test it further.

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

Successfully merging this pull request may close these issues.

2 participants