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

TypeScript class declaration – superfluous generic parameter #472

Closed
stefanpl opened this issue Apr 15, 2021 · 5 comments
Closed

TypeScript class declaration – superfluous generic parameter #472

stefanpl opened this issue Apr 15, 2021 · 5 comments

Comments

@stefanpl
Copy link

In the dist/classes/queue.d.ts you can find this class declaration:

export declare class Queue<T = any, R = any, N extends string = string> extends QueueGetters {
// …
}

But the generic parameter R is never actually used. This makes using the class a bit awkward, since (if you want to provide a custom argument for N) you have to provide a meaningless dummy value for R. That is, after you've found out that is has no real purpose in the first place.

Am I missing something here? Or should R be dropped altogether?

@manast
Copy link
Contributor

manast commented Apr 15, 2021

It is used for the job's return data type https://github.com/taskforcesh/bullmq/blob/master/src/classes/queue.ts#L64

@manast manast closed this as completed Apr 15, 2021
@stefanpl
Copy link
Author

stefanpl commented Apr 15, 2021

@manast thanks for responding so quickly. The <R> might be used in your sources files – in the dist folder that's distributed (again, please look atdist/classes/queue.d.ts) via npm it's definitely not:

add(name: N, data: T, opts?: JobsOptions): Promise<Job<any, any, string>>;

So when I do this:

const queue = new Queue<number, string>(name);
const job = queue.add("my job", 12);

my job is typed as Promise<Job<any, any, string>>. No surprise, if you check the declaration file.

Sorry for not being able to dig any deeper right now. But I definitely think this is a bug and would kindly ask you to re-open.

@manast
Copy link
Contributor

manast commented Apr 15, 2021

yes, I can see what you mean. But what is the bug? the code seems correctly to me, a bug in Typescript perhaps? if you know why the .d.ts is ignoring the R generic type please share it with me.

@stefanpl
Copy link
Author

I'm in a bit of a hurry today … hopefully I'll be able to have a closer look tomorrow or over the weekend. Nothing beats debugging weird TS stuff on a Sunday afternoon 😊

@stefanpl
Copy link
Author

Hi @manast ,

just had another look. The problem is based on the if branch inside the add function. By returning addNextRepeatableJob, which has no generic typing, TS falls back to the most generic type it can infer for the whole add function.

I added a PR to address this. Would be great if you found the time to check it out.

As a side note: For eslint, there's the explicit-module-boundary-types check, which would have prevented this kind of problem. May I ask if you have any plans to migrate to eslint?

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

No branches or pull requests

2 participants