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

@overload doesn't check implementation against overload signatures #52367

Closed
DanielRosenwasser opened this issue Jan 23, 2023 · 2 comments · Fixed by #52474
Closed

@overload doesn't check implementation against overload signatures #52367

DanielRosenwasser opened this issue Jan 23, 2023 · 2 comments · Fixed by #52474
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 23, 2023

#51234 introduced a new @overload tag, but it doesn't have the same type-checking rules as TypeScript's overloads.

Namely that we ensure that for each overload, the implementation signature is assignable to that overload, allowing the return type to be bidirectionally assignable.

If we want the same results in checkJs, then the following should error:

// @ts-check

/**
 * @overload
 * @param {number} x
 * @returns {number}
 */

/**
 * @param {string} x
 * @returns {string}
 */
function wut(x) {
    return String(x);
}

wut(123).toFixed(2);
@RyanCavanaugh
Copy link
Member

Is this desirable? I feel like our implementation/overload checking logic is so full of holes that it's not clear that JS Doc users would really benefit from it.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 23, 2023
@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Jan 23, 2023

Do you think JSDoc users want stricter checking or looser checking? If stricter, we can tighten the rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants