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

Argument becomes implicit any when a void-returning overload follows a non-void-returning signature #48088

Open
JoshuaKGoldberg opened this issue Mar 2, 2022 · 9 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@JoshuaKGoldberg
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

overload argument type signature

πŸ•— Version & Regression Information

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

declare const it: {
    (test: () => Promise<void>): void;
    (test: (input: string) => void): void;
}

it((input) => {
    input.toUpperCase();
});

it(async () => {
    // ..
});

πŸ™ Actual behavior

Error: Parameter 'input' implicitly has an 'any' type.

πŸ™‚ Expected behavior

No type error. The input parameter is type string and should be resolved as such.

For more context, see DefinitelyTyped/DefinitelyTyped#59075.

@MartinJohns
Copy link
Contributor

I think this is due to: https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#implicit-any-error-raised-for-un-annotated-callback-arguments-with-no-matching-overload-arguments

Is the first overload really needed? The code will behave the same without it.

@JoshuaKGoldberg
Copy link
Contributor Author

I think the overloads are needed -- it's either them or an overly long and confusing conditional generic type. The test function can either take in an input or return a Promise. Adjusting the snippet a little to look more like the Mocha types [playground link]:

declare const it: {
    (test: () => Promise<void>): void;
    (test: (done: () => void) => void): void;
}

it((done) => {
    done();
});

it(async () => {
    // ...
});

// @ts-expect-error
it(async (done) => {
    done();
});

I.e. you can't provide an async function that also takes in & calls a done parameter.

@fatcerberus
Copy link

fatcerberus commented Mar 2, 2022

Assuming the inference worked, you wouldn't get the expected error on the third call (the only reason it's an error now is because it's an implicit any, like the first call), because it would still match the second overload, per https://github.com/microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void:

declare const it: {
    (test: () => Promise<void>): void;
    (test: (done: () => void) => void): void;
}

it((done: () => void) => {
    done();
});

it(async () => {
    // ...
});

it(async (done: () => void) => {
    done();
});

Any return type is assignable to a return type of void, including promises. And you couldn't prevent this at runtime either because you have no way to know that the user passed in an async function without actually calling it and observing its return value.

@JoshuaKGoldberg
Copy link
Contributor Author

the only reason it's an error now is because it's an implicit any, like the first call

Ha, that's fair.

But I think this issue is still valid, that the becoming an implicit any is an undesirable behavior (dating back a while) because now there's no way to accurately represent the Mocha types.

@fatcerberus
Copy link

fatcerberus commented Mar 2, 2022

A bit of a tangent but () => void accepting any return type is usually desirable (see FAQ link above), but I do sometimes wish we could write () => undefined for the few cases where you really want to exclude value returns and not then be forced to write an explicit return undefined;.

@RyanCavanaugh RyanCavanaugh added Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript labels Mar 3, 2022
@RyanCavanaugh
Copy link
Member

I'm not quite sure what can be done here. Overload resolution behavior is extremely baked-in and it's going to be tough to change it. PRs that don't break other stuff are welcome πŸ˜…

@JoshuaKGoldberg
Copy link
Contributor Author

Hmm, now that #29374 is in I was looking forward to making another multi-year pull request. Great...!

@fatcerberus
Copy link

@RyanCavanaugh I thought this issue wasn’t about overload resolution, so much as contextual typing of callback parameters failing in the mere presence of overloads.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Mar 3, 2022

The problem is that we assign a contextual type during overload resolution, and don't have an acceptable way to "undo" that. Prior attempts to fix this have failed for one reason or another.

See also #11936

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants