Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs] Update demos to TypeScript #1990

Closed
2 tasks done
OnkelTem opened this issue Jul 13, 2020 · 12 comments
Closed
2 tasks done

[docs] Update demos to TypeScript #1990

OnkelTem opened this issue Jul 13, 2020 · 12 comments
Milestone

Comments

@OnkelTem
Copy link

OnkelTem commented Jul 13, 2020

  • The issue is present in the latest release (v.3)
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

Project cannot be compiled due to a TS error.

Expected Behavior 馃

No error.

Steps to Reproduce 馃暪

https://codesandbox.io/s/autumn-pond-6n2p7?file=/src/App.tsx

As you may see, there are errors in the 14, 22 and 29 lines.
They don't prevent project from running on that sandbox, but locally it fails to build.

TypeScript error in .../my-app/src/App.tsx(15,9):
Type 'Dispatch<SetStateAction<Date | null>>' is not assignable to type '(date: MaterialUiPickersDate) => void'.
  Types of parameters 'value' and 'date' are incompatible.
    Type 'MaterialUiPickersDate' is not assignable to type 'SetStateAction<Date | null>'.
      Type 'Moment' is not assignable to type 'SetStateAction<Date | null>'.
        Type 'Moment' is missing the following properties from type 'Date': toDateString, toTimeString, toLocaleDateString, toLocaleTimeString, and 35 more.  TS2322

    13 |         inputVariant="outlined"
    14 |         value={selectedDate}
  > 15 |         onChange={handleDateChange}
       |         ^
    16 |       />
    17 | 
    18 |       <DateTimePicker

I followed the installation guide and opted to moment.js as the utils lib:

npm i @date-io/moment@1.x moment

Context 馃敠

I try to get the first example from the docs working, to understand how it's assumed to be used at all.

Your Environment 馃寧

Tech Version
@material-ui/core v4.11.0
@material-ui/pickers v3.2.10
React ^16.9.0
Browser
TypeScript
etc.
@OnkelTem OnkelTem added the status: needs triage These issues haven't been looked at yet by a maintainer. label Jul 13, 2020
@oliviertassinari oliviertassinari added docs typescript and removed status: needs triage These issues haven't been looked at yet by a maintainer. labels Jul 13, 2020
@oliviertassinari
Copy link
Member

@OnkelTem Thanks for the feedback. We do no longer support v3, we would encourage you to have a look at https://next.material-ui-pickers.dev/.

Also, I think that we can use this as an opportunity to update all the demos to TypeScript (once we move the source into the main repo, we will get the JavaScript version automatically generated.

@oliviertassinari oliviertassinari added this to the v5 milestone Jul 13, 2020
@oliviertassinari oliviertassinari changed the title Docs are still incoherent and don't provide a clear plan on how to install [docs] Update demos to TypeScript Jul 13, 2020
@dmtrKovalenko
Copy link
Member

I think here is a conflict between date-fns and moment adapter. It is actually the same problem as described here https://next.material-ui-pickers.dev/guides/typescript#important-note-on-type-inference

Actually we need not to only update demos, but also autogenerate demos for different adapters

@oliviertassinari
Copy link
Member

but also autogenerate demos for different adapters

Linking #1650 as related.

@OnkelTem
Copy link
Author

@oliviertassinari

Good, I'm gonna try the next version then.

@OnkelTem
Copy link
Author

OnkelTem commented Jul 14, 2020

Switched to the version 4.x (4.0.0-alpha.9) and right away I get an error:

import { LocalizationProvider } from '@material-ui/pickers';

// pick an adapter for your date library
import MomentUtils from '@material-ui/pickers/adapter/moment';

// ...

<LocalizationProvider dateAdapter={ MomentUtils }>
                      ^^^^^^^^^^^
TS2322: Type 'typeof MomentUtils' is not assignable to type 'new (...args: any) => MuiPickersAdapter<unknown>'.
 聽聽Type 'MomentUtils' is missing the following properties from type 'IUtils<unknown>': yearFormat, yearMonthFormat, dateTime12hFormat, dateTime24hFormat, and 12 more.

  LocalizationProvider.d.ts(6, 5): The expected type comes from property 'dateAdapter' which is declared here on type 'IntrinsicAttributes & LocalizationProviderProps & { children?: ReactNode; }'

Ideas?
Should I create a new ticket?

@OnkelTem
Copy link
Author

OnkelTem commented Jul 14, 2020

Omg, folks, that was not reliable: I couldn't reproduce the previous problem in a new clean project.

Ignore everything from below (I forgot to type useState())

But instead, I could reproduce the initial problem, but now - with 4.x branch.

Checkout this sandbox: https://codesandbox.io/s/pickers-next-onchange-rqpvq?file=/src/App.tsx

<DatePicker
        value={selectedDate}
        onChange={handleDateChange}
        ^^^^^^^^
        renderInput={props => <TextField {...props} />}
      />

As you see there's that same error another error, but again - with onChange:

(JSX attribute) BasePickerProps<ParsableDate<React.SetStateAction<Date>>, Date | ((prevState: Date) => Date) | null>.onChange: (date: Date | ((prevState: Date) => Date) | null, keyboardInputValue?: string | undefined) => void
onChange callback @DateIOType.

Type 'Dispatch<SetStateAction<Date>>' is not assignable to type '(date: Date | ((prevState: Date) => Date) | null, keyboardInputValue?: string | undefined) => void'.
  Types of parameters 'value' and 'date' are incompatible.
    Type 'Date | ((prevState: Date) => Date) | null' is not assignable to type 'SetStateAction<Date>'.
      Type 'null' is not assignable to type 'SetStateAction<Date>'.ts(2322)
BasePicker.d.ts(11, 5): The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & BaseDatePickerProps & Pick<ResponsiveWrapperProps, "displayStaticWrapperAs" | ... 11 more ... | "desktopModeMediaQuery"> & BasePickerProps<...> & Pick<...> & WithDateAdapterProps<...> & RefAttributes<...>'

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 14, 2020

@OnkelTem Well, this confusion is IMHO a good argument as to why we need to bring TypeScript demos (#1990) and isolated demos (#1650) that can open in CodeSandbox. This alone, I think will be a significant differentiator with the alternative React date pickers.

As a developer, when you are evaluating different options, you have limited time to experiment with each of them.

@oliviertassinari oliviertassinari added the important Need work label Jul 14, 2020
@OnkelTem
Copy link
Author

OnkelTem commented Jul 15, 2020

There's also one problem with the pickers in WebStorm. The DatePicker properties are not resolved.

Usually one can Ctrl-click on any property of a component and get into its definition. With DatePicker (and DateTimePicker) all its properties are not clickable and forcing Ctrl-click on any of them brings an error hint: Cannot find declaration to go to.
It's not the case however with LocalizationProvider.

Any ideas?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 15, 2020

Which version are you using? Do you have a reproduction with codesandbox 4.0.0-alpha.9?

@OnkelTem
Copy link
Author

@oliviertassinari No, I cannot reproduce the problem with properties not being resolved in CodeSandbox. I can create a ticket however in the WebStorm issue queue about this problem.

Also, I think it's worth trying the same in VS Code.

@oliviertassinari
Copy link
Member

@OnkelTem Cool, in this case, we can ignore it. Let's focus on updating all the demos to TypeScript, this increases the TypeScript test coverage at the same time than helping new users.

@oliviertassinari
Copy link
Member

It was fixed in https://next.material-ui.com/components/pickers/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants