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] Cannot pass options to the customParseFormat dayjs plugin #11148

Closed
n0rb opened this issue Nov 22, 2023 · 4 comments · Fixed by #11151
Closed

[pickers] Cannot pass options to the customParseFormat dayjs plugin #11148

n0rb opened this issue Nov 22, 2023 · 4 comments · Fixed by #11151
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module!

Comments

@n0rb
Copy link

n0rb commented Nov 22, 2023

Steps to reproduce

Steps:

  1. use a <LocalizationProvider dateAdapter={AdapterDayjs}>
  2. create a global config for dayjs in a mydayjs.ts file
import dayjs from 'dayjs';
import customParseFormat from 'dayjs/plugin/customParseFormat';

dayjs.extend(customParseFormat, {
  parseTwoDigitYear: yearString => +yearString + 1800
})

3.test the override somewhere in your app below the LocalizationProvider:

// Page.tsx
const mydate = dayjs('98-04-25', 'YY-MM-DD').format('YYYY-MM-DD');
console.log('test date', date); 

Current behavior

the test will output
1998-04-25

Expected behavior

it should output
1898-04-25
due to parseTwoDigitYear override

Context

my customer needs custom parsing of two digit year inputs. ie all two digit inputs should be parsed to 1800s

this can be established with dayjs customParseFormat Plugin and parseTwoDigitYear Function override
iamkun/dayjs#1418
iamkun/dayjs#1421

(the default behavior of dayjs with two digit years is

let parseTwoDigitYear = function (input) {
  input = +input
  return input + (input > 68 ? 1900 : 2000)
}

however when using AdapterDayjs it already extends Dayjs at line 18 https://github.com/mui/mui-x/blob/master/packages/x-date-pickers/src/AdapterDayjs/AdapterDayjs.ts

dayjs does not seem to allow extending again with customParseFormat plugin and setting parseTwoDigitYear is ignored

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: AdapterDayjs parseTwoDigitYear customParseFormat

@n0rb n0rb added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 22, 2023
@michelengelen michelengelen self-assigned this Nov 23, 2023
@michelengelen
Copy link
Member

Hey @n0rb
Thanks for raising this issue. 🙇🏼
It indeed looks like the extend we call prevents another extend with options.
I'll open up a quick PR. 🚢

@michelengelen michelengelen added bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 23, 2023
@flaviendelangle
Copy link
Member

flaviendelangle commented Nov 23, 2023

One solution would be to move our extend to the constructor of the adapter, that way we are sure that it's applied after the user one.
We could use dayjs.p.customParseFormat to know if it has already been applied before re-applying it, because I could imagine issues with plugins applied twice given how day.js treat them.

But that is a breaking change because people could rely on the existence of this plugin before any component is rendered.

@michelengelen
Copy link
Member

One solution would be to move our extend to the constructor of the adapter, that way we are sure that it's applied after the user one. We could use dayjs.p.customParseFormat to know if it has already been applied before re-applying it, because I could imagine issues with plugins applied twice given how day.js treat them.

But that is a breaking change because people could rely on the existence of this plugin before any component is rendered.

Yes. That's basically what I was investigating just now as well. I did also try to find a way if you can add the option after extending, but that does not seem possible

@michelengelen
Copy link
Member

Interestingly enough we don't need that check before the call to extend(), because dayjs does check that on its own. Thats the reason why it did not work in the first place.

dayjs.extend = (plugin, option) => {
  if (!plugin.$i) { // install plugin only once
    plugin(option, Dayjs, dayjs)
    plugin.$i = true
  }
  return dayjs
}

So if the plugin has already been applied it will just return the instance without doing anything on it.

So we can safely call extend() in the constructor. But you are right, this is indeed a breaking change.

@flaviendelangle flaviendelangle changed the title AdapterDayjs: Cannot override parseTwoDigitYear of dayjs customParseFormat [pickers] AdapterDayjs: Cannot override parseTwoDigitYear of dayjs customParseFormat Nov 23, 2023
@flaviendelangle flaviendelangle changed the title [pickers] AdapterDayjs: Cannot override parseTwoDigitYear of dayjs customParseFormat [pickers] Cannot pass options to the customParseFormat dayjs plugin Nov 23, 2023
@michelengelen michelengelen removed their assignment Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants