SimonErm / react-native-job-queue

Easy to use react-native queuing library
https://simonerm.github.io/react-native-job-queue/
122 stars 33 forks source link

[FTR]: Worker.remove()? #47

Closed myckhel closed 3 years ago

myckhel commented 3 years ago

Given the below snippet.

useEffect(() => {
    queue.addWorker({
        name: 'message.upload',
        worker: () => {}
    });
    queue.addWorker({
        name: 'story.upload',
        worker: () => {}
    });
    queue.start();

    return () => {
      queue.removeWorker('message.upload');
      queue.removeWorker('story.upload');
    };
  }, []);

i would like to remove the worker without hardcoding the worker name.

Would it be a good idea for addWorker() to return a worker instance?

if so then we can add a remove method to to Worker which will call queue.removeWorker(this.name) rather than the hardcode.

Finally would look like the snippet below

useEffect(() => {
    const storyUploadWorkerInstance = queue.addWorker({
        name: 'message.upload',
        worker: () => {}
    });
    const messageSenderWorkerInstance = queue.addWorker({
        name: 'story.upload',
        worker: () => {}
    });
    queue.start();

    return () => {
      storyUploadWorkerInstance.remove()
      messageSenderWorkerInstance.remove()
    };
  }, []);

Let me have your say. Thanks.

SimonErm commented 3 years ago

Would this bring any benefits? Because that would introduce a depedency from th worker to the queue.

myckhel commented 3 years ago

The benefit would be for the fact that it will return a worker instance which we can directly invoke call remove on instead of manually calling queue.removeWorker(workerName);

if we cant afford to add worker dependency to the queue, maybe we could try to make addWorker return a remove function.

addWorker(worker: Worker<any>) {
    if (this.workers[worker.name]) {
        throw new Error(`Worker "${worker.name}" already exists.`);
    }
    this.workers[worker.name] = worker;

    return () => this.removeWorker(worker.name)
}

finally the usage could be like this:

useEffect(() => {
  const removeMsgWorker = queue.addWorker({
      name: 'message.upload',
      worker: () => {}
  });
  const removeStoryWorker = queue.addWorker({
      name: 'story.upload',
      worker: () => {}
  });
  queue.start();

  return () => {
    removeMsgWorker()
    removeStoryWorker()
  };
}, []);

How about this idea?

SimonErm commented 3 years ago

I still don't see a real benefit, but the second approach would be a nice to have. I would prefer if the addWorker method would return the workerName, because than it could be used similar to setTimeout/clearTimout etc. Furthermore there is still the removeRelatedJobs flag, which could be passed. What do you think?

SimonErm commented 3 years ago

Feel free to reopen this.