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

JSDoc @overloads on constructors don't capture class type parameters, can dictate their return type #55919

Open
DanielRosenwasser opened this issue Sep 29, 2023 · 7 comments
Assignees
Labels
checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Needs Investigation This issue needs a team member to investigate its status.

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 29, 2023

Found from #55916.

Today if you try to write a generic constructor, captured type parameters fail to create a generic signature. Instead, the type parameter is effectively leaked and never instantiated.

/**
 * @template T
 */
class C {
    /**
     * @overload
     * @param {T} value
     */

    /**
     * @overload
     * @param {T} first
     * @param {T} second
     */

    /**
     * @param {T} first
     * @param {T} [second]
     */
    constructor(first, second) {
    }
}

const x = new C(123);
//              ~~~
// Argument of type 'number' is not assignable to parameter of type 'T'.
//  'T' could be instantiated with an arbitrary type which could be unrelated to 'number'.

const y = new C("hello", "world");
//              ~~~~~~~
// Argument of type 'string' is not assignable to parameter of type 'T'.
//  'T' could be instantiated with an arbitrary type which could be unrelated to 'string'.

The current-workaround is to write something like the following:

/**
 * @template T
 */
class C {
    /**
     * @template T
     * @overload
     * @param {T} value
     * @return {C<T>}
     */

    /**
     * @template T
     * @overload
     * @param {T} first
     * @param {T} second
     * @return {C<T>}
     */

    /**
     * @param {T} first
     * @param {T} [second]
     */
    constructor(first, second) {
    }
}

const x = new C(123);
//    ^?

const y = new C("hello", "world");
//    ^?

const y = new C(123, "hello");
//    ^?

Notice that here, each @overload must declare its own type parameters and return type.

We should decide if this is actually how we want @overload to work in JSDoc on constructor declarations.

@DanielRosenwasser DanielRosenwasser added Needs Investigation This issue needs a team member to investigate its status. Domain: JSDoc Relates to JSDoc parsing and type generation checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically labels Sep 29, 2023
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 5.4.0 milestone Sep 29, 2023
@DanielRosenwasser
Copy link
Member Author

I'll point out that in TS itself, construct signatures can't add more generic type parameters than their containing class.

It's an irregularity that overload signatures permit them in the JSDoc version.

@ITenthusiasm
Copy link

ITenthusiasm commented Oct 14, 2023

Some extra notes on this topic:

  1. Currently (for the workaround example), all of the type parameters seem to leak from the class and from the constructor overloads. So to get generic constructors working as expected with JSDocs, each type parameter for each overload has to be distinct (e.g., T1, T2, T3).
  2. The workaround mentioned works perfectly during development. However, the generated .d.ts files are somehow invalid to TypeScript. Within the generated .d.ts file itself, I see the error: Cannot find name 'TYPE_PARAMETER'. So it seems the workaround may not work for production builds -- unless I'm missing something.

Some thoughts on generic constructors:

If people need to make sure that the generic type parameters have distinct constraints for each overload (which is my use case), then the workaround syntax might actually be desirable. Technically speaking, it's the constructor that's generic in each use case rather than the class itself. There are times when the constructor needs to behave differently depending on its inputs, but the constructor will produce the same API as an output regardless. In these cases, the class is technically not generic.

I know generic constructors aren't currently supported in TypeScript, but I think #40451 gives a compelling case for their use. They're technically already doable with interfaces. But the developer experience with interfaces is a little difficult since the JSDocs used to document the code end up being separated from the actual class definition. (JSDocs on the class definition that receives the constructor interface type are ignored. Only JSDocs on the interface type are acknowledged.)


I wasn't sure if any of these comments needed to be split out into separate issues. If they do, please let me know and I can split them out.

@sandersn
Copy link
Member

(1) and (2) need to be tested in the fix for this bug.

Your thoughts on generic constructors would be good to have on #40451; JS isn't supposed to support generic constructors until TS does.

@ITenthusiasm
Copy link

@sandersn For clarity, do you mean my thoughts on #40451 or @DanielRosenwasser's?

@ITenthusiasm
Copy link

Added a comment to that issue just in case. I have time to write comments down now so I figured I'd just take advantage of the opportunity if that was the desire. 😅

@sandersn
Copy link
Member

sandersn commented Nov 2, 2023

Oh, I meant it would be good to write down your thoughts from here, on #40451. Basically exactly what you did; sorry I was unclear.

@ITenthusiasm
Copy link

I know I linked it in the other issue, but just in case the reference to my use case is needed here as well: https://github.com/enthusiastic-js/form-observer/blob/main/packages/core/FormObserver.js

ITenthusiasm added a commit to enthusiastic-js/form-observer that referenced this issue Nov 26, 2023
Because TypeScript does not yet support generic
constructors for JSDocs, this change is necessary
in order to guarantee a decent user experience
for the users of `@form-observer/core`.

See:
- microsoft/TypeScript#55919
- microsoft/TypeScript#40451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
checkJs Relates to checking JavaScript using TypeScript Domain: JavaScript The issue relates to JavaScript specifically Domain: JSDoc Relates to JSDoc parsing and type generation Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

3 participants