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

[DatePicker] Needs to be generic over TDate #5388

Closed
2 tasks done
Philipp91 opened this issue Dec 13, 2020 · 9 comments
Closed
2 tasks done

[DatePicker] Needs to be generic over TDate #5388

Philipp91 opened this issue Dec 13, 2020 · 9 comments
Labels

Comments

@Philipp91
Copy link

Philipp91 commented Dec 13, 2020

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

Current Behavior 馃槸

When I do:

import StaticDateRangePicker, {StaticDateRangePickerProps} from '@material-ui/lab/StaticDateRangePicker';

then StaticDateRangePickerProps does not know the TDate, i.e. it uses unknown. This means that simple things like this don't work:

const MyPicker: React.FC<MyPickerProps> = ({value, ...rest}) => <StaticDateRangePicker value={value} {...rest}/>;

TypeScript rightfully complains that the value prop coming into my component is of type RangeInput<unknown> and thus not assignable to the StaticDateRangePicker.value prop, which is of type RangeInput<Date>. Similar issues exist with minDate, maxDate and onChange.

Expected Behavior 馃

StaticDateRangePickerProps should be generic <TDate = unknown> or even <TDate = Date>, allowing me to use the props type as usual and customize it when necessary.

Steps to Reproduce 馃暪

I tried putting together a live example, but I don't know how to import the latest alpha of the lab there.

Context 馃敠

I'm trying to wrap the built-in component in a custom one which converts dates to local time in some way, adds some default property values, etc.

Your Environment 馃寧

`npx @material-ui/envinfo`
npx: installed 2 in 9.29s

  System:
    OS: Linux 4.4 Ubuntu 18.04.5 LTS (Bionic Beaver)
  Binaries:
    Node: 14.0.0 - ~/.nvm/versions/node/v14.0.0/bin/node
    Yarn: 1.10.1 - /usr/bin/yarn
    npm: 6.14.9 - ~/.nvm/versions/node/v14.0.0/bin/npm
  Browsers:
    Chrome: Not Found
    Firefox: Not Found
  npmPackages:
    @material-ui/core: ^5.0.0-alpha.18 => 5.0.0-alpha.18
    @material-ui/icons: ^5.0.0-alpha.15 => 5.0.0-alpha.15
    @material-ui/lab: ^5.0.0-alpha.18 => 5.0.0-alpha.18
    @material-ui/styled-engine:  5.0.0-alpha.18
    @material-ui/styles:  5.0.0-alpha.18
    @material-ui/system:  5.0.0-alpha.18
    @material-ui/types:  5.1.0
    @material-ui/unstyled:  5.0.0-alpha.18
    @material-ui/utils:  5.0.0-alpha.18
    @types/react: ^17.0.0 => 17.0.0
    react: ^17.0.1 => 17.0.1
    react-dom: ^17.0.1 => 17.0.1
    typescript: ^4.1.3 => 4.1.3
tsconfig
{
  "compilerOptions": {
    "baseUrl": "./src",
    "target": "esnext",
    "moduleResolution": "node",
    "allowJs": true,
    "noEmit": true,
    "isolatedModules": true,
    "esModuleInterop": true,
    "jsx": "react",
    "sourceMap": true,
    "experimentalDecorators": true,
    "noImplicitAny": true,
    "alwaysStrict": true,
    "strict": true
  },
  "include": [
    "src"
  ]
}

@Philipp91 Philipp91 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 13, 2020
@oliviertassinari
Copy link
Member

Related to mui/material-ui-pickers#2045.

@oliviertassinari oliviertassinari added component: DateRangePicker The React component. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 13, 2020
@Philipp91
Copy link
Author

So the two solutions are (a) dragging TDate all the way out through all components, or (b) assuming TDate=Date everywhere and hard-coding it, thus getting rid of the generics.

What's your current opinion/plan on this?

@oliviertassinari
Copy link
Member

In https://codesandbox.io/s/staticdaterangepicker-material-demo-forked-j9y1x?file=/demo.tsx, the type seems correct. Could you expand?

Capture d鈥檈虂cran 2020-12-27 a虁 22 55 17

@Philipp91
Copy link
Author

You're right, it didn't work because I had overridden some props assuming TDate-Date.

Let me reformulate as a feature request: I want to be able to provide (from my "application library" to my "application code") a DatePicker component that uses Date as opposed to unknown for its value, so that there's no awkward type casting in the application code. (Then I also want to wrap/customize that component, but that's actually a different story.)

So I imagine something like export const MyDatePicker = MuiDatePicker<Date>.

@oliviertassinari oliviertassinari added component: DatePicker The React component. and removed component: DateRangePicker The React component. labels Dec 29, 2020
@oliviertassinari oliviertassinari changed the title [DateRangePicker] Needs to be generic over TDate [DatePicker] Needs to be generic over TDate Dec 29, 2020
@oliviertassinari
Copy link
Member

@Philipp91 Ok, so the issue isn't specific to the date range picker but also ranges to the date picker. This sounds like a good idea. Do you want to work on it?

@Philipp91
Copy link
Author

That's right, it's not specific to the range picker. Sadly I haven't found the time to work on it, and likely I won't. In my application, I can work around the issue with a couple of as casts and a wrapper component that I have anyway (for Formik).

@flaviendelangle flaviendelangle transferred this issue from mui/material-ui Jul 4, 2022
@flaviendelangle
Copy link
Member

Fixed by #4617
All pickers now have two generics TInputDate and TDate

We will write a doc page to fully document it in the future

@DanielSchranc
Copy link

@flaviendelangle Hi, is it possible to share the link to documentation for this change of additional generics TinputDate and TDate I would like to know what is the intention and purpose of it and how it suppose to be used? Thank you 馃檶

@flaviendelangle
Copy link
Member

Hi,

Sorry for the delay
You have the description of the generics on #4617

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

No branches or pull requests

4 participants