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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pickers] Support AdapterLuxon with throwOnInvalid = true #11853

Open
harry-peirse-jfc opened this issue Jan 29, 2024 · 8 comments
Open

[pickers] Support AdapterLuxon with throwOnInvalid = true #11853

harry-peirse-jfc opened this issue Jan 29, 2024 · 8 comments
Labels
component: pickers This is the name of the generic UI component, not the React module! support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ waiting for 👍 Waiting for upvotes

Comments

@harry-peirse-jfc
Copy link

harry-peirse-jfc commented Jan 29, 2024

Steps to reproduce

Link to live example

Steps:

  1. Run tsc --noEmit in the terminal
  2. Error appears

Current behavior

It is standard practice in Luxon to write the following when invalid dates should throw errors:

import { Settings } from "luxon";

Settings.throwOnInvalid = true;

declare module "luxon" {
  interface TSSettings {
    throwOnInvalid: true;
  }
}

In particular this type override tells Luxon that null won't be returned when parsing DateTimes.

Unfortuantely when this is set, it now breaks the AdapaterLuxon type checking when providing it to the LocalizationProvider. This worked without issues on @mui/x-date-pickers-pro version 6.18.4

Expected behavior

Providing AdapterLuxon to the LocalizationProvider with throwOnInvalid configured to true should not cause type errors.

Context

No response

Your environment

npx @mui/envinfo
 Browser:
    Chrome, but issue is browser agnostic
 System:
    OS: Linux 6.1 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
  Binaries:
    Node: 20.9.0 - /usr/local/bin/node
    npm: 9.8.1 - /usr/local/bin/npm
    pnpm: 8.10.2 - /usr/local/share/npm-global/bin/pnpm
  Browsers:
    Chrome: Not Found
  npmPackages:
    @emotion/react: latest => 11.11.3 
    @emotion/styled: latest => 11.11.0 
    @mui/base:  5.0.0-beta.33 
    @mui/core-downloads-tracker:  5.15.6 
    @mui/material: latest => 5.15.6 
    @mui/private-theming:  5.15.6 
    @mui/styled-engine:  5.15.6 
    @mui/system:  5.15.6 
    @mui/types:  7.2.13 
    @mui/utils:  5.15.6 
    @mui/x-date-pickers: latest => 6.19.2 
    @mui/x-date-pickers-pro: latest => 6.19.2 
    @mui/x-license-pro:  6.10.2 
    @types/react: latest => 18.2.48 
    react: latest => 18.2.0 
    react-dom: latest => 18.2.0 
    typescript: latest => 5.3.3 

Search keywords: DateTime Typescript AdapterLuxon Error
Order ID: 60670

@harry-peirse-jfc harry-peirse-jfc added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 29, 2024
@zannager zannager added component: pickers This is the name of the generic UI component, not the React module! support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ labels Jan 29, 2024
@michelengelen michelengelen changed the title Type Error with AdapterLuxon [pickers] Type Error with AdapterLuxon Jan 29, 2024
@michelengelen
Copy link
Member

Hey @harry-peirse-jfc I can confirm this behavior (bug? 🤷🏼).
I'll put this on the board for the team to evaluate the effort!
Thanks! 🙇🏼

@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 30, 2024
@andbjer
Copy link

andbjer commented Feb 22, 2024

This issue was introduced in v6.18.7. I believe this is the PR that introduced the issue #11566

@oscar-b
Copy link

oscar-b commented Jun 14, 2024

This is easily reproduced by deleting for instance the month part (in the textfield) of a <DateTimePicker<DateTime>>

Uncaught Error: Invalid DateTime: invalid input
    at DateTime.invalid (datetime.js:940:13)
    at DateTime.fromJSDate (datetime.js:600:23)
    at AdapterLuxon.getInvalidDate (AdapterLuxon.js:208:77)
    at clearActiveSection (useFieldState.js:158:69)
    at eval (useFieldV6TextField.js:283:9)
    at eval (useEventCallback.js:25:19)
    at handleChange (InputBase.js:396:7)

Or entering a date in the textfield of a <DateTimePicker<DateTime>> which is initialised without a date:

Uncaught Error: Invalid DateTime: unparsable: the input "0002 NaN NaN hh mm" can't be parsed as format yyyy M d HH mm
    at DateTime.invalid (datetime.js:940:13)
    at parseDataToDateTime (datetime.js:185:21)
    at DateTime.fromFormat (datetime.js:890:14)
    at AdapterLuxon.parse (AdapterLuxon.js:229:58)
    at getDateFromDateSections (useField.utils.js:334:16)
    at updateSectionValue (useFieldState.js:210:99)
"@mui/material": "^5.15.16",
"@mui/x-date-pickers": "^7.7.0",

@LukasTy
Copy link
Member

LukasTy commented Jun 14, 2024

@oscar-b @andbjer @harry-peirse-jfc I've created a draft PR with an experiment on how to handle this luxon setting.
Consider trying the following package: https://pkg.csb.dev/mui/mui-x/commit/064c2340/@mui/x-date-pickers to test the behavior and provide any feedback if possible.

-"@mui/x-date-pickers": "^7.7.0"
+"@mui/x-date-pickers": "https://pkg.csb.dev/mui/mui-x/commit/064c2340/@mui/x-date-pickers"

The main issue/question is how do we handle these "invalid" thrown dates as currently it seems more as a bug than a feature.
Blurring the text field hides the input/query, when the entry is invalid.
Also, IMHO, it would be logical to put the field in an error state when the entry is "invalid/thrown".

Screen.Recording.2024-06-14.at.17.49.33.mov

What would you consider a proper behavior given the Settings.throwOnInvalid = true?
I'll remind you that this setting messes up with the expected lifecycle behavior of Fields.
We loose the intermediate Invalid date state. 🙈

@oscar-b
Copy link

oscar-b commented Jun 17, 2024

@LukasTy Thanks for looking into this. The whole situation with types in Luxon is.. not optimal. I don't know if you did more research, but here's some interesting tickets:
DefinitelyTyped/DefinitelyTyped#64995 DefinitelyTyped/DefinitelyTyped#65078
moment/luxon#1421

Your PR seems to do the best with the situation at hand (only checked your video, haven't tried it myself yet), but losing the date on blur probably won't fly here. I'll probably have to investigate disabling throwOnInvalid in our codebase instead, considering the issues around Luxon wrt this.

@LukasTy
Copy link
Member

LukasTy commented Jun 19, 2024

@oscar-b thank you for sharing the discussion links.
I can only add that if luxon went for a separate import path (luxon/strict) approach, that could be even harder to support as opposed to the existing solution.

I'm not a fan of this behavior (throwOnInvalid) from the Picker Adapters perspective, because luxon is the only library/adapter, that I'm of, which has such option.
All the other libraries (and even JS Date) just create an object, which can be identified as invalid and we rely on this.
Properly supporting luxon.throwOnInvalid means creating special checks and cases just to cater to this one implementation case. 🙈 🤷

Is the lack of support on the @mui/x-date-pickers side for luxon.throwOnInvalid a deal-breaker for anyone?
Or can we wait and see what direction the library takes?

@oscar-b
Copy link

oscar-b commented Jun 21, 2024

@LukasTy For our case, I'm reverting to not using throwOnInvalid anymore, so we're good. Hopefully luxon will add support for locally specifying throwOnInvalid overriding the global setting, but I'm not holding my breath.

@LukasTy LukasTy added the waiting for 👍 Waiting for upvotes label Jun 21, 2024
@LukasTy LukasTy changed the title [pickers] Type Error with AdapterLuxon [pickers] Support AdapterLuxon with throwOnInvalid = true Jun 21, 2024
@SaebAmini
Copy link

After a confused long search of why my build was suddenly encountering the below error, I luckily found this thread linked from the documentation page to understand why it's happening.

I'll just paste the error here to help others find this thread from search engines:

<LocalizationProvider dateAdapter={AdapterLuxon}> results in the type error:

Types of construct signatures are incompatible.
  Type 'new ({ locale, formats }?: { formats?: Partial<AdapterFormats> | undefined; locale?: string | undefined; } | undefined) => AdapterLuxon' is not assignable to type 'new (...args: any) => MuiPickersAdapter<DateTime<true>, string>'.
    Construct signature return types 'AdapterLuxon' and 'MuiPickersAdapter<DateTime<true>, string>' are incompatible.
      The types returned by 'date(...)' are incompatible between these types.
        Type 'DateTime<true> | DateTime<false> | null' is not assignable to type 'DateTime<true> | null'.
          Type 'DateTime<false>' is not assignable to type 'DateTime<true>'.
            Type 'false' is not assignable to type 'true'.ts(2419)
LocalizationProvider.d.ts(22, 5): The expected type comes from property 'dateAdapter' which is declared here on type 'IntrinsicAttributes & LocalizationProviderProps<DateTime<true>, string>'
(property) LocalizationProviderProps<DateTime<true>, string>.dateAdapter?: (new (...args: any) => MuiPickersAdapter<DateTime<true>, string>) | undefined
Date library adapter class function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ waiting for 👍 Waiting for upvotes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants