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

Default parameter narrowed to be defined when using void | undefined | SomeType to type the function parameter #48664

Closed
dummdidumm opened this issue Apr 13, 2022 · 5 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@dummdidumm
Copy link

Bug Report

When typing a function parameter so that it's optional by typing it as void | SomeType method and defining a default parameter, the type is not narrowed to SomeType and instead stays as void | SomeType. TS can be tricked into doing so by writing void | undefined | SomeType, but this feels inconsistent.

From what I've read, void means "I promise I won't use this return value" in the context of fnThatAccecptsVoidFn in the code snippet below, so the return value can actually be anything, which means a default parameter in fnWithDefaultParam1/2 maybe cannot be used to narrow the function parameter to SomeType.

This is either ok because wo broke our promise of not using the return value of the function in fnThatAccecptsVoidFn - in this case void | SomeType with a default value should also be narrowed to SomeType. Or TS plays it safe and says "if void is accepted, anything could be passed in" and not narrow void | undefined | SomeType to SomeType.

Before you say "you could just make this an optional function parameter using ?": This is the minimum reproducible, the actual code is in the context of sveltejs/svelte#7442 where we try to type out a function interface that makes it possible to either set a parameter as required, optional, or missing, where I found this (in my mind) inconsistency. This is related to #12400.

🔎 Search Terms

function default parameter void undefined

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about void, function default parameter

⏯ Playground Link

https://www.typescriptlang.org/play?#code/MYewdgzgLgBAZmA6gSygCxAVygEQKZwCGmANlAAqEBOhAtjALwwAUADtXQFwwBuIyAExgAfGAG9C3KFUx4AvgEpGAPnEAoGJpjsatAHSEA3DAD0J+IWQkIAGhh4AHqzzAoeAWrlq1oSLAQo6PhEpBQctACMjCw6XLz8QqISUjLy0WIwkjDSsjCKKupa2uEGxmYWVrYwmGCOzq7uAPye3r7Q8EioaMHEZJS6AEzRbOHcfIIi1WACBMi1ieJZOWlMGUupeUoMqmIaWrH6RqbmAO4gVADWVXUubgLNXj7g7QgAKmiEUACCwMAurFAIAA1BIAMTAwwQ3GYW1U4wEsMKWgCXR6oX6dAGzAQMIUZXMVDwUEwVAhBxgwEIEIARngYNA0PZrHT0FTxJkUrk5DBzlMZnA5u4Wmo3h9vr9-oCQYJwcwYQVlni1EA

💻 Code

const fnWithoutDefaultParam = (param: void | {a: true}) => {
    param.a; // fails, expected
}

const fnWithDefaultParam1 = (param: void | {a: true} = { a: true }) => {
    param.a; // fails, unexpected?
}

const fnWithDefaultParam2 = (param: void | undefined | {a: true} = { a: true }) => {
    param.a; // works, expected?
}

const fnThatAccecptsVoidFn = (fn: () => void) => {
    fnWithDefaultParam2(fn()); // return param can be sth else than { a: true } or undefined. Type-hole that is ok because we break our promise of not using the return type?
}

fnThatAccecptsVoidFn(() => true);

🙁 Actual behavior

void | SomeType + a default parameter don't narrow it to SomeType, but void | undefined | SomeType do, which feels inconsistent. See first section for more details.

🙂 Expected behavior

Honestly I don't know, see sections above.

@MartinJohns
Copy link
Contributor

MartinJohns commented Apr 13, 2022

Or TS plays it safe and says "if void is accepted, anything could be passed in"

That is precisely what void means: It can be any value and it should not be used for anything. That means a type like void | SomeType does not make much sense. void does not mean absence of a value (aka undefined). This is due to this behaviour.

An exmaple:

function doSomething(): { a: number } {
    return { a: 123 };
}

function callMeMaybe(callback: () => void) {
    return callback();
}

function fnWithVoidParam(param: void | { a: true }) {
    if (param) {
        // You'd expect boolean, but is actually number.
        console.log(typeof param.a);
    }
}

fnWithVoidParam(callMeMaybe(doSomething));

Granted, the behaviour of TypeScript is weird here. It actually does narrow the type, when it shouldn't. There are many issues about the behaviour of void, like #42709.


You should use undefined instead, which is a specific value.

@dummdidumm
Copy link
Author

I know that void means "can be any value" in this example, and in the code snippet I've shown a variation of your example why I think it's unsafe. I also think it's a bug that void | undefined | SomeType + default value narrows it, but there could be arguments against aligning it with the void | SomeType and instead make void | SomeType + default value narrow it, too - hence my "don't know what the expected behavior is". I can't use undefined | SomeType because that doesn't make the parameter optional ("function expects 2 arguments, got 1"). This is tracked in #12400, would be great if that lands at some point.

@RyanCavanaugh
Copy link
Member

We'd really, really, really like to just ban using void like this, but can't because people took a hard dependency on doing so back in the pre-strictNullChecks days when this was a quasi-working hack to emulate undefined, plus some weirdness around Promises. The rules in place are heuristics that try to be as safe as possible while not breaking old code and are not expected to be consistent.

For this particular scenario, have you considered using a conditional type + a parameter tuple type?

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Apr 13, 2022
@dummdidumm
Copy link
Author

dummdidumm commented Apr 14, 2022

We considered the following (simplified a little)

export type Action<Element = HTMLElement, Parameter = any> = 
	void extends Parameter
	? <Node extends Element>(node: Node) => void
	: undefined extends Parameter
	? <Node extends Element>(node: Node, parameter?: Parameter) => void
	: <Node extends Element>(node: Node, parameter: Parameter) => void;

but I just noticed that this won't work as expect when strict null checks is not turned on because undefined extends Parameter is always true then. So I'm honestly not sure how else to solve the requirement that people should be able to type that a second parameter is either missing, optional, or required without introducing extra concepts like "use ThisSpecificTypeThatTellsUsItsOptional | SomeType when you want the 2nd param to be optional".

Sorry if this type inconsistency issue becomes a support ticket 😅

@github-actions
Copy link

github-actions bot commented Jun 8, 2023

This issue has been marked as 'Not a Defect' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

3 participants