Skip to content

Optional parameters should reject undefined parameter values #10014

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

Closed
falsandtru opened this issue Jul 28, 2016 · 9 comments
Closed

Optional parameters should reject undefined parameter values #10014

falsandtru opened this issue Jul 28, 2016 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@falsandtru
Copy link
Contributor

For example, a following signature of function f should be defined like function g with strictNullChecks option.

TypeScript Version: master

Code

function f(a?: number) {
}
f();
f(0);
f(undefined);

function g()
function g(a: number)
function g(a?: number) {
}
g();
g(0);
g(undefined);

Expected behavior:

$ node built/local/tsc.js --strictNullChecks index.ts
index.ts(5,3): error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'number'.
index.ts(13,3): error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'number'.

Actual behavior:

$ node built/local/tsc.js --strictNullChecks index.ts
index.ts(13,3): error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'number'.
@sandersn
Copy link
Member

I don't think this make sense:

  1. strictNullChecks defines a?: number as equivalent to a?: number | undefined.
  2. Javascript defines f(undefined) as equivalent to f()

(1) means that by definition, under --strictNullChecks, all ?-suffixed arguments admit undefined. So f(undefined) is fine. Additionally, (2) means that this is the correct behaviour for compatibility with Javascript.

If anything, g(undefined) should be legal and be equivalent to g().

@falsandtru
Copy link
Contributor Author

Thanks for the explanation. I agree with you if it is TypeScript's policy. However, (2) has different behavior.

> !function () {console.log(arguments.length)}()
0
true
> !function () {console.log(arguments.length)}(undefined)
1
true

And it can be discriminated by overloads. Additionally, if optional parameters can reject undefined, TypeScript can check the parameter length by parameter checks such as if (a === void 0) without seeing arguments.length.

@falsandtru
Copy link
Contributor Author

@sandersn More, TypeScript shouldn't allow mixed parameter when calling.

function f(a?: number, b?: number) {
}
f(undefined, 0); // bad style, should be an error

It is not allowed as a type.

// index.ts(1,24): error TS1016: A required parameter cannot follow an optional parameter.
function f(a?: number, b: number) {
}

So optional parameters should reject undefined parameter values.

@sandersn
Copy link
Member

sandersn commented Aug 2, 2016

I don't think the similarity between function definition and calling holds. You don't want to penalize a caller for calling an ugly function. So f(undefined, 9) looks like an acceptable workaround for calling a function that overuses optional parameters.

The fact remains that the type of optional parameters includes undefined, so it would be surprising if you couldn't pass undefined to them.

@sandersn sandersn added the Suggestion An idea for TypeScript label Aug 2, 2016
@falsandtru
Copy link
Contributor Author

falsandtru commented Aug 2, 2016

So f(undefined, 9) looks like an acceptable workaround for calling a function that overuses optional parameters.

I believe it should use a data structure and destructuring assignment, this is a basic practice to deal partial data.

function f({a = 0, b}: { a?: number; b: number; }) {
}
f({a: undefined, b: 1});

related: #10030

@sandersn
Copy link
Member

sandersn commented Aug 2, 2016

I agree that f should use a data structure, but the caller may not necessarily be able to change it. In that case we are penalizing the caller for using a badly written function. This seems like a good candidate for a lint rule, used for policing a code base that is entirely owned by one project. (And it would be easy to write as well)

@falsandtru
Copy link
Contributor Author

I'm very glad if TypeScript supports this penalization by option like --noInitializingParameter.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 19, 2017

undefined is the value at run time of an optional parameter that is not specified. it seems pointless to try to hide this fact from the user. I think this scenario is covered by overloads.. you can define the function f above with two overloads, one with no parameters and one with two required parameters.. when you use two optional parameters, you clearly indicate that both can be undefined independently.

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Suggestion An idea for TypeScript labels Oct 19, 2017
@falsandtru
Copy link
Contributor Author

This is an actual problem but this solution is also unrealistic. This is a design limitation of optional parameters.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants