Skip to content

Function argument comparison doesn't match expectations #20541

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
alexburner opened this issue Dec 7, 2017 · 8 comments
Closed

Function argument comparison doesn't match expectations #20541

alexburner opened this issue Dec 7, 2017 · 8 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@alexburner
Copy link

TypeScript Version: 2.6.1-insiders.20171019 (via TS Playground)

Code

const addOne = (
  effect: (
    arg1: number,
    arg2: () => 'A'
  ) => void
): void => {
  effect(1, () => 'A');
};

addOne(
  (
    arg1: number,
    arg2: (cheese: 'wheel') => void,
  ) => console.log(arg1 + 1)
);

Expected behavior:
Type error about arg2 mismatch, indicating that (cheese: 'wheel') => void is incompatible with () => 'A'.

Actual behavior:
No type error.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Dec 7, 2017
@RyanCavanaugh
Copy link
Member

@ghost
Copy link

ghost commented Dec 7, 2017

Situations like these show why it's often a good idea to omit type annotations where possible. If you just write addOne((arg1, arg2) => arg2(arg1)) you will get an error because arg2 doesn't need any arguments.

@alexburner
Copy link
Author

@RyanCavanaugh whoops, I should RTFM, those two sections address this exactly. I could understand the Substitutability argument for these cases, proving that nothing terrible will happen.

However, in our use case, it would still be preferable that a type error is thrown, since the mismatch of arguments reveals that a developer on our team is likely mis-using the tool. It would be nice if there was a compiler flag like --strictFunctionSignatureMatching that users could opt into for additional guard rails.

@Andy-MS that's very interesting less explicit types add more strictness, I'm not sure I understand why that is. I've been assuming the more type annotations I add, the safer I am...

@alexburner
Copy link
Author

One issue I have with that FAQ description, it says:

Second, let's consider another case:

let items = [1, 2, 3];
items.forEach(arg => console.log(arg));

This is isomorphic to the example that "wanted" an error. At runtime, forEach invokes the given callback with three arguments (value, index, array), but most of the time the callback only uses one or two of the arguments. This is a very common JavaScript pattern and it would be burdensome to have to explicitly declare unused parameters.

But couldn't the forEach callback just use optional arguments?

(value?, index?, array?) => void

@ghost
Copy link

ghost commented Dec 7, 2017

No, because marking a parameter as optional means it might not be provided. But forEach always provides those, so they are non-optional (and you can rely on index not being undefined).

I've been assuming the more type annotations I add, the safer I am

If you have --noImplicitAny enabled (part of --strict), then we will tell you any time you need to add a type annotation. So you don't have to worry about some un-annotated thing being any.

@RyanCavanaugh
Copy link
Member

See #17868 and others linked from that issue

@RyanCavanaugh
Copy link
Member

I've amended the FAQ since this is a common counter-question

@alexburner
Copy link
Author

Ohhhhhhhh @Andy-MS I see my misunderstanding, I was missing this:

No, because marking a parameter as optional means it might not be provided. But forEach always provides those, so they are non-optional (and you can rely on index not being undefined).

And thanks @RyanCavanaugh for the link, I had been unable to search up a relevant issue. Your comment in there helps me understand:

This isn't what an optional parameter means! If strictly enforced (which we don't because everyone gets it wrong when writing declaration files), an optional parameter in a callback position means "I (the caller) might not provide this parameter", not "You (the callee) are free to ignore this parameter".

Also @Andy-MS that makes sense about the --noImplicitAny flag meaning I can lean more heavily on type inference!

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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

2 participants