Skip to content

React .tsx optional function arguments aren't optional #26369

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
benneq opened this issue Aug 10, 2018 · 3 comments
Closed

React .tsx optional function arguments aren't optional #26369

benneq opened this issue Aug 10, 2018 · 3 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@benneq
Copy link

benneq commented Aug 10, 2018

TypeScript Version: 3.0.1

Search Terms:
react, tsx, function, argument, arguments, parameter, parameters, optional, undefined

Code

// MyComponent.tsx
interface MyComponentProps {
    onChange: (value?: number) => void
}


// ... inside some other component's render function:
// MyOtherComponent.tsx
const handleChange = (value: number) => { ... }

const someTypeScriptFunction = (value?: number) => {
  handleChange(value); // ERROR
}

<MyComponent onChange={handleChange} /> // NO ERROR

Expected behavior:
Both (MyComponent's onChange and someTypeScriptFunction) should behave the same, because they have the same argument types.

Actual behavior:
The compiler allows to call handleChange from the react template.
When doing mouseover in VSCode, I can see that onChange adapts the type of handleChange. VSCode says: onChange: (value: number) => void.
And when I change the argument of handleChange to value?: number, then VSCode suddenly says onChange: (value?: number | undefined) => void.

Playground Link:
The editor doesn't display an error for someTypeScriptFunction, but my VSCode does. But you still can see the changing type of onChange, which depends on the argument type of handleChange.
https://codesandbox.io/s/4q4n73w50w

Related Issues:
Maybe this is related: #26004 (but I'm not 100% sure)

@mattmccutchen
Copy link
Contributor

You are running into function parameter bivariance. The proper analogy would be this:

// MyComponent.tsx
interface MyComponentProps {
    onChange: (value?: number) => void
}

class MyComponent extends React.Component<MyComponentProps> {}

// ... inside some other component's render function:
// MyOtherComponent.tsx
const handleChange = (value: number) => {  };

function someTypeScriptFunction(props: MyComponentProps) {}

someTypeScriptFunction({onChange: handleChange});  // 1
<MyComponent onChange={handleChange} />  // 2

If you enable strictNullChecks and strictFunctionTypes, then both 1 and 2 give an error. If you enable strictNullChecks but disable strictFunctionTypes, then neither 1 nor 2 gives an error.

I agree it's confusing that hovering over onChange in 2 shows (value: number) => void while hovering over onChange in 1 shows (value?: number | undefined) => void. Indeed, my proposed approach to fixing #26004 may have the side effect of fixing this.

Given that the function parameter bivariance is working as intended, shall we repurpose this issue for the inconsistency in hover information?

@benneq
Copy link
Author

benneq commented Aug 11, 2018

Oh, damn. You're right. I had disabled strictFunctionTypes in my tsconfig.

If your fix for #26004 also fixes the hover information, then I guess this issue can be closed.
Else if you want to keep the issue to track "inconsistency in hover information", feel free to keep it open and/or rename it.

Thank you! :)

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Aug 13, 2018
@typescript-bot
Copy link
Collaborator

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes. If you're still waiting on a response, questions are usually better suited to stackoverflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants