0 is not acceptable by value

[jiggum]

Environment

Tech	Version
@material-ui/pickers	3.2.10
material-ui	4.9.2
TypeScript	3.6.3
React	16.10.2
Peer library	[email protected]

Steps to reproduce

Using TimePicker with value of 0

Expected behavior

Picker should use value of 0

Actual behavior

Using time of now

Description

I think the below line is omitting the value of 0
https://github.com/mui-org/material-ui-pickers/blob/next/lib/src/_helpers/date-utils.ts#L106

const parsedValue = utils.date(value || defaultHighlight || now);

[dmtrKovalenko]

It should not be acceptable. If you need 00:00 time set value={startOfDay(new Date())}

[oliviertassinari]

I wonder if we have a prop-type warning in this case. @jiggum Do you have a live reproduction?

[oliviertassinari]

It doesn't. @dmtrKovalenko should we have a custom prop-type to detect this class of issues? Based on the Material-UI stats, 90% of the developers are using the JavaScript demos, in the user-survey, it's 55% without a typing system.

[dmtrKovalenko]

I wonder that will be to complicated. Because we accept any value that is parsable by any date library. So any value that acceptable by any library is acceptable by us https://dev.material-ui-pickers.dev/getting-started/parsing

[oliviertassinari]

@dmtrKovalenko Ok, it's good to know, so maybe there is no DX improvement opportunity 🤷‍♂️.

[jiggum]

@dmtrKovalenko Thank you for example. Actually we using addDays(value, 1) when the value is 0.
Can I know the reason why 0 is should not be acceptable?
In Js's Date object, 0 means Unix time's base timestamp. So it had been a little confused to me when the actual input value is current time rather than 0.

[oliviertassinari]

@jiggum Could you provide a reproduction? What date engine are you using? How does this date engine parse 0?

[jiggum]

@oliviertassinari We only depend on typescript's prop typing and there's no type warning.

What date engine are you using?

We using date-fns.
When we parse '00:00:00' at UTC timezone, parse(time, 'HH:mm:ss', 0).valueOf() is return 0

the third parameter(0) is refrenceDate if we set that value to another date, above return another number which is not 0.

In formatting part, we using format(time, 'HH:mm:ss') and it return 00:00:00

Here is that case
https://codesandbox.io/s/date-fns-v2-ndpjq

[dmtrKovalenko]

Probably it’s a pitfall if || operator and we should use ?? instead. But it is adding some overhead in runtime

[oliviertassinari]

@dmtrKovalenko We have started using ?? in mui/material-ui#20715. But != null would be as good, it's the approach we use most of the time.

[dmtrKovalenko]

Yes we are using it already. But it adds a bit bundle size overhead.

[oliviertassinari]

I think that if @jiggum is able to provide a valid reproduction, we could spend more time on it.

Should we consider a version like

export function parsePickerInputValue(
  now: MaterialUiPickersDate,
  utils: MuiPickersAdapter,
  { value, defaultHighlight }: Pick<BasePickerProps, 'value' | 'defaultHighlight'>
): MaterialUiPickersDate | null {
  return [value, defaultHighlight, now].reduce((acc, item) => {
    const candidate = utils.date(item);

    if (!acc && utils.isValid(candidate)) {
      return candidate;
    }

    return acc;
  }, null);
}

instead of

export function parsePickerInputValue(
  now: MaterialUiPickersDate,
  utils: MuiPickersAdapter,
  { value, defaultHighlight }: Pick<BasePickerProps, 'value' | 'defaultHighlight'>
): MaterialUiPickersDate | null {
  const parsedValue = utils.date(value || defaultHighlight || now);

  return parsedValue && utils.isValid(parsedValue) ? parsedValue : now;
}

?

[jiggum]

@oliviertassinari Is reproduction means something like this?
https://codesandbox.io/s/material-ui-date-picker-dyyhp

[oliviertassinari]

@jiggum Almost, here is what I was looking for https://codesandbox.io/s/material-ui-date-picker-gk7d0 (v4.0.0-alpha.5) :).

The input value is correct, but the displayed value in the popup is wrong.