Types require onChange to be defined

[RebeccaStevens]

Environment

Tech	Version
@material-ui/pickers	v3.2.2
@material-ui/core	v4.9.5
TypeScript	v3.8.2
React	v16.12.0

Steps to reproduce

Use KeyboardDatePicker without defining onChange, onBlur or onFocus:

import { KeyboardDatePicker } from '@material-ui/pickers';
import React, { FunctionComponent } from 'react';

type Props = { name: string; value: Date; };

const Element: FunctionComponent<Props> = ({ name, value }) => (
  <KeyboardDatePicker name={name} value={value} />
);

Expected behavior

No error messages (onChange, onBlur and onFocus should be optional).

Actual behavior

Error Message when only onChange missing:

Property 'onChange' is missing in type '{ name: string; value: Date; }' but required in type 'BaseKeyboardPickerProps'.

Error Message when only onBlur missing:

Property 'onBlur' is missing in type '{ name: string; value: Date; onChange: (date: MaterialUiPickersDate) => void; onFocus: () => void; }' but required in type 'Pick<KeyboardDateInputProps, "ref" | "label" | "select" | "style" | "title" | "mask" | "className" | "innerRef" | "key" | "defaultChecked" | "defaultValue" | ... 273 more ... | "rifmFormatter">'.

Error Message when only onFocus missing:

Property 'onFocus' is missing in type '{ name: string; value: Date; onChange: (date: MaterialUiPickersDate) => void; onBlur: () => void; }' but required in type 'Pick<KeyboardDateInputProps, "ref" | "label" | "select" | "style" | "title" | "mask" | "className" | "innerRef" | "key" | "defaultChecked" | "defaultValue" | ... 273 more ... | "rifmFormatter">'.

Live example

https://codesandbox.io/s/elated-platform-swzeq

[dmtrKovalenko]

onChange is required and must be required – there is no sense in datepicker if you are not handling its value.
But onBlur and onFocus are not required - https://codesandbox.io/s/peaceful-sun-df3ux

Closing as invalid

[oliviertassinari]

@dmtrKovalenko I believe that:

1. if the input is "disabled" or "readonly", providing an onChange doesn't make sense.
2. there is a use case for server-side form submission.
3. react-hook-form might be used without an onChange handler, only looking at the value of the DOM input.
4. none of the core components force the onChange prop, it would be more consistent.

So, I think that removing such a requirement would help

[dmtrKovalenko]

@oliviertassinari I cannot agree with that,

1. react-hook-form doing completely wrong if they are attaching handlers right to the dom, because updating dom is side-effect for us when user is picker value from calendar.
2. I don't like such tricky behaviors like when there is a value and there is no onChange or inverted. When there is no onChange – always question appears how it is changing value. It is much easier to provide a do nothing function onChange={() => { }}
3. It is more "proper" and "strict" way from the type system perspective
4. Typescript must help developers to not make a mistake. If there is no onChange provided it is in 98% a mistake so we need to make it clear

[oliviertassinari]

1. if the input is "disabled" or "readonly", providing an onChange doesn't make sense.

@dmtrKovalenko What's your take on 1.?

[dmtrKovalenko]

Make <DatePicker disabled onChange={() => {}}
It is clear enough that there is no action appears on changing value, yes there are more words, but it is clear and expressive way to show that there is no action inside

[oliviertassinari]

Typescript must help developers to not make a mistake. If there is no onChange provided it is in 98% a mistake so we need to make it clear

@dmtrKovalenko From my perspective, it's nor ours call to make, it would be best to stick to the typing (prop-types) React uses for its input. onChange isn't required? Fine, so shouldn't it be for @material-ui/core nor @material-ui/lab, etc.

[dmtrKovalenko]

@oliviertassinari f9r text-field maybe. For Datepicker - definitely no

[RebeccaStevens]

Sorry for my bad with onBlur and onFocus. This issue I was having with them seems to already be fixed in v3.2.10, I was using v3.2.2 (can't believe I forgot to check whether or not I was using the latest version).

Typescript must help developers to not make a mistake.

@dmtrKovalenko I don't fully agree with this.
TypeScript should just provide type information to developers; this in turn then helps developers to not make mistakes.

If the KeyboardDatePicker works fine without the onChange prop being defined (i.e. the underlying code can handle the case of onChange === undefined), then TypeScript should not enforce it to be defined.

[oliviertassinari]

TypeScript should just provide type information to developers; this in turn then helps developers to not make mistakes.

In our case, the prop types and TypeScript types should match, right?

[dmtrKovalenko]

Yes and proptype for onChange in picker should be required

Can’t agree about only type information. I think that we must force users to avoid mistakes

[RebeccaStevens]

@dmtrKovalenko In my opinion, it should be a linter that enforces onChange be set; not TypeScript itself.