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

Minor type checking improvement for cluster.queue method #70

Closed
cyxou opened this issue Dec 12, 2018 · 3 comments
Closed

Minor type checking improvement for cluster.queue method #70

cyxou opened this issue Dec 12, 2018 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@cyxou
Copy link

cyxou commented Dec 12, 2018

First of all, thank you for your work!
I have a minor suggestion on improving type checking fot the cluster.queue() method.
Now we have this:

public async queue(
        data: JobData | TaskFunction,
        taskFunction?: TaskFunction,
    ): Promise<void> {
...
}

As one can inspect, JobData is of type any and it is used both as a first argument to cluster.queue() method as well as the data property of the TaskFunctionArguments interface. This approach does not provide sufficient type checking when we call the cluster.queue() method with two arguments. I'd suggest to use generic types here like this:

type QueueFunction<T> = (arg: QueueFunctionArguments<T>) => Promise<void>;

interface QueueFunctionArguments<T> {
  page: puppeteer.Page;
  data: T;
  worker: {
    id: number;
  };
}

public async queue<T>(
    data: T | TaskFunction,
    taskFunction?: QueueFunction<T>,
): Promise<void> {
...
}
@thomasdondorf thomasdondorf added the enhancement New feature or request label Dec 21, 2018
@thomasdondorf thomasdondorf mentioned this issue Dec 21, 2018
15 tasks
@thomasdondorf
Copy link
Owner

Looks good. As this might break implementations, I'll put this into the roadmap for v1 (#8).

@thomasdondorf thomasdondorf added this to the v1.0 milestone Dec 21, 2018
@thomasdondorf
Copy link
Owner

This could actually be implemented without being a breaking change by using any as default for the generics.

@thomasdondorf
Copy link
Owner

I just implemented this. You can now specify the types for input/output via Cluster<input, output>. output is used when execute is called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants