Typescript (seemingly) choosing the wrong overload-CodePudding

I have a method like this:

class Foo {
    public runInWorker(cb: () => void): void;
    public runInWorker(cb: () => Promise<void>): void | Promise<void>;
    public runInWorker(cb: () => void | Promise<void>): void | Promise<void> {
        if (cluster.isPrimary ?? cluster.isMaster) {
            return;
        }

        return cb();
    }
}

What I want to do is to pass in a sync or async function and I want the overload to return the correct type (i.e. void for a sync function and Promise<void> for an async function).

This works fine for a sync callback:

// result is void
const result = new Foo().runInWorker(() => {})

But it fails with an async callback:

// result is void, but it should be Promise<void>
const result = new Foo().runInWorker(async () => {})

How do I fix this issue?

CodePudding user response：

I'm not sure precisely why, but a function returning a promise of void is type-convertible to a function returning void (I suppose because the return value of a function returning void should never be used, so a function A-which-returns-non-void with compatible parameter signature to function B is compatible with B if / even if B returns void). The opposite is not true:

const ok: () => void = async () => {};
const notOk: () => Promise<void> = () => {};

The TypeScript handbook section on function overloads says:

In order for the compiler to pick the correct type check, it follows a similar process to the underlying JavaScript. It looks at the overload list and, proceeding with the first overload, attempts to call the function with the provided parameters. If it finds a match, it picks this overload as the correct overload. For this reason, it’s customary to order overloads from most specific to least specific.

As I mentioned at the beginning, your overload which takes a sync callback and returns void can accept a sync or async callback. The overload which takes an async callback can only accept an async callback and not a sync callback, which makes it the "more specific" overload.

So to fix your problem, I'm pretty sure you can just reorder your overloads as so:

class Foo {
    public runInWorker(cb: () => Promise<void>): void | Promise<void>;
    public runInWorker(cb: () => void): void;
    public runInWorker(cb: () => void | Promise<void>): void | Promise<void> {
        return cb();
        // ...

CodePudding user response：

Use generics instead

class Foo {
  public runInWorker<T>(cb: () => T): T {
    return cb();
  }
}

CodePudding user response：

First up, the problem is probably this line:

    public runInWorker(cb: () => Promise<void>): void | Promise<void>;

If you pass a function returning a promise, the return value should be a promise, so change it to

    public runInWorker(cb: () => Promise<void>): Promise<void>;

However, I would argue that the design is bad. You should either work with callbacks or promises, not both. I.e. it's fine to have a function that can take either a callback as input and return nothing - or take no callback as input, and return a promise. But callbacks that return promises is a strange mix.

So I could deal with this in this way

public run(cb: (err: Error, data: Data) => void): void;
public run(): Promise<Data>;
public run(cb?: (err: Error, data: Data) => void): void | Promise<Data> {
  // In this version, the function is natively implemented with callbacks,
  // so we wrap that in a promise here.
  if (!cb) {
    return new Promise((resolve, reject) => {
        this.run((err, data) => err ? reject(err) : resolve(data))
    })
  }

  // Here the rest of the function that natively works with a callback
}