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

In JS, jsdoc should be able to declare functions as overloaded #25590

Closed
sandersn opened this issue Jul 11, 2018 · 43 comments · Fixed by #51234
Closed

In JS, jsdoc should be able to declare functions as overloaded #25590

sandersn opened this issue Jul 11, 2018 · 43 comments · Fixed by #51234
Labels
Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically
Milestone

Comments

@sandersn
Copy link
Member

Search Terms

jsdoc overloads

Suggestion

You can specify overloads in Typescript by providing multiple signatures. You should be able to do the same thing in Javascript. The simplest way I can think of is to allow a type annotation on a function declaration.

Examples

/** @typedef {{(s: string): 0 | 1; (b: boolean): 2 | 3 }} Gioconda */

/** @type {Gioconda} */
function monaLisa(sb) {
    // neither overloads' return types are assignable to 1 | 2, so two errors should be logged
    return typeof sb === 'string' ? 1 : 2;
}

/** @type {2 | 3} - call should resolve to (b: boolean) => 2 | 3, and not error */
var twothree = monaLisa(false);

// js special property assignment should still be allowed
monaLisa.overdrive = true;

Note that checking parameters against the implementation parameters will be trivial since there's no obvious type but any for the implementation parameters. Checking of return types against the implementation return type will have to use the return type of the function body as the implementation return type. That check will also likely be trivial since function-body return types usually depend on parameters.

@mhegazy mhegazy added Salsa Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript labels Jul 11, 2018
@mhegazy mhegazy added this to the TypeScript 3.1 milestone Jul 11, 2018
@AlCalzone
Copy link
Contributor

How would one provide comments and parameter descriptions (/** @param */) when doing this?
Sometimes it is necessary to highlight the differences between overloads. In .d.ts files this works by annotating each single overload with its own comment. See also #407

@AlCalzone
Copy link
Contributor

One way I could think of is

/**
 * Does something
 * @callback foo_overload1
 * @param {number} arg1 Something
 */

 /**
 * Does something else
 * @callback foo_overload2
 * @param {string} arg1 Something else
 * @param {number} arg2 Something
 */

/** @type {foo_overload1 | foo_overload2} */
function foo(arg1, arg2) {

}

but this way, we get an error on the type annotation that the signatures don't match.

@AlCalzone
Copy link
Contributor

Too bad this seems pretty low on your priorities :( I just ran into an issue where it is impossible to document a method signature in JavaScript due to this. I have a function where all arguments except the last one are optional:

// valid overloads:
declare function test(arg1: string, arg2: number, arg3: () => void): void;
declare function test(arg2: number, arg3: () => void): void;
declare function test(arg1: string, arg3: () => void): void;
declare function test(arg3: () => void): void;

I'd be really glad if documenting this via JSDoc would be possible in the near future.

@sandersn
Copy link
Member Author

Here's a type that is too permissive, but it's pretty close:
declare function test(arg123: string | number | () => void, arg23?: number | () => void, arg3?: () => void): void

In order to express dependencies between types without overloads, you need conditional types:

declare function test<T extends string | number | () => void>(
    arg123: T, 
    arg23?: T extends string ? number : 
            T extends number ? () => void : 
            never, 
    arg3?: T extends string ? () => void : never): void

But conditional type syntax doesn't work in JS so you'd have to write a series of type aliases in a .d.ts and reference them.

@AlCalzone
Copy link
Contributor

AlCalzone commented Nov 26, 2018

@sandersn Thanks for the explanation, but it does not quite achieve what I'm trying to do.

I have a JS CommonJS module which contains said function with a couple of overloads similar to the ones I mentioned above. Inside the module, this function is heavily used so I want to give it proper type checking and document the parameters. The function is not exported.

The project maintainer does not use TypeScript, but lets me work with JSDoc comments to get type checking support. Asking him to move every overloaded function into separate modules and add .d.ts files for them is too much to ask.

When annotating the function with @type {(arg1: union | of | types, ...) => void}, the type checker fails to understand the type of variables inside the function and unnecessarily complains about seemingly disallowed assignments. Also, I cannot provide additional explanation to the single parameters like I would do in an overloaded declaration + JSDoc @param comments.

The external .d.ts file does not work, even if I hack something like this:

// file1.d.ts
export declare function test(arg1: string, arg2: number, arg3: () => void): void;
export declare function test(arg2: number, arg3: () => void): void;
export declare function test(arg1: string, arg3: () => void): void;
export declare function test(arg3: () => void): void;
// ^ the function is NOT exported

// file1.js
/** @type {typeof import("./file1").test} */
// ^ ERROR: The type of a function declaration must match the function's signature. [8030]
function test(arg1, arg2, arg3) {
    return;
}

I could live with the signature being to permissive, but in this situation, I'm unable to document the parameter types without "breaking" the implementation.

@weswigham weswigham added Domain: JavaScript The issue relates to JavaScript specifically and removed Domain: JavaScript The issue relates to JavaScript specifically Salsa labels Nov 29, 2018
@sandersn
Copy link
Member Author

Another suggestion from the VS user voice: https://developercommunity.visualstudio.com/content/idea/425961/support-3.html

@AlCalzone
Copy link
Contributor

I can't believe this does not get more feedback. This is a pain in the a** to work with in legacy codebases with overloaded callback-style APIs.

@ahocevar
Copy link

Unfortunately, the workarounds described here no longer work due to #44693.

@RyuGuo
Copy link

RyuGuo commented Sep 21, 2021

You can try this:

/**
 * @type {{
 * (s:string) => 1;
 * (b:boolean) => 2;
 * }}
 */
const monaLisa = (sb) => {
    return typeof sb === 'string' ? 1 : 2;
}
let two = monaLisa(false);   // vscode can recognize that this is 2
let one = monaLisa("m");     // this is 1

You can not use function monaLisa() to declare it. I think this is a defect.
In class, you can:

class A {
    /**
     * @type {{
     * (s:string) => string;
     * (n:number) => number;
     * }}
     */
    func = (sn) => {}
}
let n = (new A()).func(0);
let s = (new A()).func("s");

But, A.func is a property, not a method in vscode.

@jespertheend
Copy link
Contributor

I'm doing (string, callback) => {} type overloading like this:

/**
 * @typedef {Object} CallbacksMap
 * @property {MouseEvent} click
 * @property {DragEvent} drag
 */

/**
 * @template {keyof CallbacksMap} T
 * @param {T} eventType The identifier of the event type.
 * @param {function(CallbacksMap[T]) : void} cb The callback to invoke when the event occurs.
 */
addEventListener(eventType, cb) {
    // ...
}

@jespertheend
Copy link
Contributor

The cleanest way to do this that I've found so far is using the Parameters type.
This allows you to declare functions like you usually do, like function functionName() {}, rather than var functionName = function() {}.
And on classes you can declare methods like normally rather than this.methodName = () => {}

/**
 * This function takes a number and a string.
 * @callback signatureA
 * @param {number} num
 * @param {string} str
 */

/**
 * This function takes a boolean and an object
 * @callback signatureB
 * @param {boolean} bool
 * @param {Object} obj
 */

/**
 * @param {Parameters<signatureA> | Parameters<signatureB>} args
 */
function overloaded(...args) {}

playground link

Unfortunately the function description doesn't get copied, but hopefully #46353 fixes that.

@mhofman
Copy link

mhofman commented Nov 15, 2021

I recently tried to type the following templated overload with JSDoc, without success. Resorted to a .d.ts declaration and @type the export.

declare function processFoo<T>(foo: Foo, transform: (bar: BarOfFoo) => Promise<T>): Promise<T>;
declare function processFoo(foo: Foo): Promise<DefaultBarOfFoo>;

@systemmonkey42
Copy link

I recently tried to type the following templated overload with JSDoc, without success. Resorted to a .d.ts declaration and @type the export.

I have been forced to do the same. It seems that each typescript/tsserver update becomes stricter. Workaround which used to work, are no longer accepted.

Where possible I just use typescript natively now.

@andiby
Copy link

andiby commented Nov 15, 2021

With a workaround it's still possible to do this with jsdoc only, but now we need to use type casting.
And with type casting it now automatically recognizes the types of the parameters:

/**
@typedef {{
    (s: string): 0 | 1
    (b: boolean): 2 | 3
    [k:string]: any
}} Gioconda
*/

const monaLisa = /** @type {Gioconda} */(
    (sb) => {
        return typeof sb === 'string' ? 1 : 2;
    }
);

const obj = {
    monaLisa: /** @type {Gioconda} */(
        (sb) => {
            return typeof sb === 'string' ? 1 : 2;
        }
    )
};

class MonaLisa {
    monaLisa = /** @type {Gioconda} */(
        (sb) => {
           return typeof sb === 'string' ? 1 : 2;
        }
    )
}

/** @type {2 | 3} - call resolve to (b: boolean) => 2 | 3 */
var twothree = monaLisa(false);
/** @type {2 | 3} - call resolve to (b: boolean) => 2 | 3 */
var twothreeObj = obj.monaLisa(false);
/** @type {2 | 3} - call resolve to (b: boolean) => 2 | 3 */
var twothreeClass = new MonaLisa().monaLisa(false);

// js special property assignment should still be allowed
monaLisa.overdrive = true;
obj.monaLisa.overdrive = true;
MonaLisa.prototype.monaLisa.overdrive = true;

class BarOfFoo {}
class Foo {}
class DefaultBarOfFoo {}

/**
@typedef {{
    <T>(foo: Foo, transform: (bar: BarOfFoo) => Promise<T>): Promise<T>
    (foo: Foo): Promise<DefaultBarOfFoo>
}} ProcessFoo
*/

const processFoo = /** @type {ProcessFoo} */(
    async(foo, transform) => {
        if (transform)
            return await transform(foo);
        return new DefaultBarOfFoo;
    }
);

/** @param {BarOfFoo} bar */
async function transformNumber(bar) {
    return 1;
}

/** @param {BarOfFoo} bar */
async function transformString(bar) {
    return 'str';
}

/** @type {Promise<number>} */
const test1 = processFoo(new Foo, transformNumber);
/** @type {Promise<string>} */
const test2 = processFoo(new Foo, transformString);
/** @type {Promise<DefaultBarOfFoo>} */
const test3 = processFoo(new Foo);

Playground Link

@mhofman
Copy link

mhofman commented Nov 15, 2021

/**
@typedef {{
    <T>(foo: Foo, transform: (bar: BarOfFoo) => Promise<T>): Promise<T>
    (foo: Foo): Promise<DefaultBarOfFoo>
}} ProcessFoo
*/

TIL you can write a function typedef this way in TS JSdoc! The type cast of the function was expected and I don't mind it in this case.

@systemmonkey42
Copy link

/**
@typedef {{
    <T>(foo: Foo, transform: (bar: BarOfFoo) => Promise<T>): Promise<T>
    (foo: Foo): Promise<DefaultBarOfFoo>
}} ProcessFoo
*/

Ouch! My brain.

Is there a reference which explains this syntax?

I found https://austingil.com/typescript-function-overloads-with-jsdoc/ which talk about it, but nothing that explains it, especially how the template works in this context.

@andiby
Copy link

andiby commented Nov 16, 2021

Inside the {...} in jsdoc we can use any TypeScript syntax. And in TypeScript this overload with a generic would look like this:

class BarOfFoo {}
class Foo {}
class DefaultBarOfFoo {}

function processFoo<T>(foo: Foo, transform: (bar: BarOfFoo) => Promise<T>): Promise<T>
function processFoo(foo: Foo): Promise<DefaultBarOfFoo>
function processFoo<T>(foo: Foo, transform?: any) : any {
}

let asType: typeof processFoo;

Playground Link

Now just place your mouse over "asType" and you get the tooltip:

let asType: {
    <T>(foo: Foo, transform: (bar: BarOfFoo) => Promise<T>): Promise<T>;
    (foo: Foo): Promise<DefaultBarOfFoo>;
}

@apendua
Copy link
Contributor

apendua commented May 5, 2022

I have came across the following (a little bit hacky) idea:

/**
 * @param {number} x
 * @returns {'number'}
 */
function typeName__1(x) {};

/**
 * @param {string} x
 * @returns {'string'}
 */
function typeName__2(x) {};

/**
 * @param {unknown} x
 * @returns {string}
 */
function typeName(x) {
  return typeof x;
}

Now if we modify the parser slightly and we teach it to interpret [name]__[number] as a body-less function declaration, it seems to work quite nicely. It only requires a couple lines of code to be changed in parseFunctionDeclaration(), i.e. something along these lines:

            let node: FunctionDeclaration;
            const match = name && /(.*)__\d+$/.exec(name.escapedText.toString());
            if (match) {
                const newName = factory.createIdentifier(match[1]);
                node = factory.createFunctionDeclaration(decorators, modifiers, asteriskToken, newName, typeParameters, parameters, type, undefined);
            }
            else {
                node = factory.createFunctionDeclaration(decorators, modifiers, asteriskToken, name, typeParameters, parameters, type, body);
            }

I tested it locally and it works quite well.

Of course this is not backwards compatible, so perhaps we could also decorate that declaration with additional JSDoc tag to control this behavior in a more granular way, e.g.

/**
 * @param {number} x
 * @returns {'number'}
 */
function /** @name typeName */ typeName__1(x) {};

This approach has a couple of nice properties:

  1. From TS perspective symbols for typeName__1 and typeName__2 are not really declared so if someone tries to use them, e.g. typeName__1(123), this will actually produce an error, which is desirable.
  2. From JS perspective these are just no-op functions which can be safely ignored.
  3. The same technique should work for method declarations.

@Cow258
Copy link
Contributor

Cow258 commented Jan 13, 2024

Successful overload by following this issue
#55056

/**
 * @see {@link GroupBy}
 * @template T
 * @template K
 * @overload
 * @param {T[]} items
 * @param {(item: T) => K} callback
 * @returns {Record<K, T>}
 */

/**
 * @see {@link GroupBy}
 * @template T
 * @template K
 * @overload
 * @param {T[]} items
 * @param {(item: T) => K} callback
 * @param {boolean} isMultiRow
 * @returns {Record<K, T[]>}
 */

function GroupBy(items, callback, isMultiRow) {
  if (!callback) throw new Error('GroupBy: callback is required')
  const obj = {}
  if (!items.length) return obj
  const mode = isMultiRow ? 'multi' : 'single'
  for (let i = 0; i < items.length; i++) {
    const key = callback(items[i], i)
    switch (mode) {
      case 'multi':
        if (!obj[key]) obj[key] = []
        obj[key].push(items[i])
        break
      case 'single':
        if (!obj[key]) obj[key] = items[i]
        break
    }
  }
  return obj
}

const items = [
  { id: 'foo', name: 'foo' },
  { id: 'bar', name: 'bar' },
]

const gAryB = GroupBy(items, item => item.id, true)
const gObjB = GroupBy(items, item => item.id)
console.log(gAryB[''][0].id)
console.log(gObjB[''].id)

image
image
image
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically
Projects
None yet
Development

Successfully merging a pull request may close this issue.