mui/material-ui-pickers

Migrate to date-fns or luxon?

dmtrKovalenko opened this issue ยท 28 comments

We have moved moment to the peer deps, and now I am considering about replacing moment at all with date-fns. ๐Ÿ˜Ž

So this just issue to consider if there are many people that using moment with this project?
How many people using date-fns?

I am just need to realize which way would be more convenient. โ“

We're using moment basically for timezone calculations with moment-timezone atm., still i'd like to completely migrate away from moment due to size implications. Besides missing time-zone support there are also a lot less languages supported in date-fns as in moment (https://github.com/moment/moment/tree/develop/locale vs https://github.com/date-fns/date-fns/blob/master/docs/i18n.md).

My take on this would be to wait till date-fns 2 reaches at least beta and then look into refactoring. A lot of things should be easy to refactor - the one thing which may require breaking changes is then handling of locales though. If i get it correctly date-fns is stateless so you have to pass the locale to every function which is locale aware.
In order to make migration easier once we switch to date-fns we could eventually introduce a locale prop, even know when still using moment.

At its current size moment is an all but dead library.
We are currently using moment but will make the switch soon.
The timezones are currently holding us back (date-fns/date-fns#180)

@sakulstra @mduijn What did you think about luxon?

not sure, tbh. I heard of it the first time ~5days ago and didn't use/test it at all.
I generally like their approach to use Intl features provided by the browser - but to me it seems like this is not sufficient in most cases so it complicates our lives as long as polyfills and browsersupport aren't there: https://moment.github.io/luxon/docs/manual/why.html

Eventually it would also make sense to reevaluate if we could just get away with the native Date and rip out the 3th party date libraries. Then users could just decide themselves if/what library they use. Personally i would just let it like it is for now and rethink later in a few months.

ingro commented

Honestly I'm using only date-fns for newer projects, but luxon could also be a worthy candidate, moment is just too big... But I'm with @sakulstra on this, the best way will be to work with the native Date by default and let the user use whatever they want for their needs of localization/parsing/format dates, should they needs to.

Recently react-day-picker did this by removing direct moment dependency and providing some default functions that still use moment but are entirely optional.

+1 for migrating off of moment, it's too heavy :/

+1 for migrating to date-fns. This is the main reason we do not use this date picker right now

@cherniavskii @alitaheri
I have an idea how we can support that. We can use utils, that was created for support jalali calendar system to fully replace dependency of moment in the project.
We can create material-ui-pickers-datefns-utils, material-ui-pickers-luxon-utils, material-ui-pickers-vanilla-utils to add ability choose the library. What do you think about that?

@dmtrKovalenko That sounds great ๐Ÿ‘ ๐Ÿ‘ Just keep in mind, not everything has been moved to utils, which is easy to solve ๐Ÿ˜

Also, we should come up with a solution for interoperability. I think we should accept only ISO strings to make sure every implementation can know what to do.

@alitaheri Maybe there is a way to allow passing any, to add ability pass library-specific date object?

@dmtrKovalenko Oh... right, you're right ๐Ÿ‘ ๐Ÿ‘

@dmtrKovalenko good idea!
What about default behavior? Or utils prop will be required?

@cherniavskii I think it should be required, we can do utils with vanilla date by default, but it can take some unused code, if somebody doesnt using tree shaking, I think it should throw some error if no utils passed.

So creating a single picker will require to import appropriate utils and pass them to picker?

Why not, we can also pass the vanilla utils by default, I think everybody using shared components to use pickers, so it would not be a problem? Or would be?
So we need to decide - add some additional maybe unnecessary code (with vanilla utils) or force to pass utils each time using pickers.

@dmtrKovalenko I would go with forcing utils to be specified. It's not a big deal. and can some a lot of people a lot of hassle, reduce the lib size and a lot more.

We're in beta, so breaking changes are expected anyway. ๐Ÿ‘ For externalizing utils and forcing others to require the utils of their choice.

@dmtrKovalenko I just wanted to make sure I understood your idea right ;)
But
Maybe there's a way to init utils once - like moment.locale() works?

+1 for abandoning moment. Native date is better in the long run, but if a library has to be used, then definitely date-fns. We use it in place of moment wherever we can.

I would vote for Intl.js to be as close to native as possible - if a dep. should be used, then one as small as posible. This was also the case for the built-in datepicker in material-ui@0.x.

count me in for date-fns

+1 for date-fns

Would really love to see date-fns!
Moment is waaaaay too huge, i want to keep it out of my build if at all possible.

I am using date-fns as well in my project. This picker is the only reason to add moment in my project. Would love to move away from that dependency.

+1 for date-fns

@ALL guys, we have added that in v1.0.0-rc.1

This has been closed, but I want to ask nevertheless.
Is there still interest in support for Luxon?
If so, I made a fork of this repo and built support for it including tests.

https://github.com/lookapanda/material-ui-pickers/blob/luxon/lib/src/utils/luxon-utils.js
https://github.com/lookapanda/material-ui-pickers/blob/luxon/lib/__tests__/utils/luxon-utils.test.js

If there is interest, I could do a PR.

@lookapanda wow, that's cool, I think we can add Luxon support :)
@dmtrKovalenko what do you think?

@lookapanda Way to go!
Thats just awesome, I have think about adding support in the next release. Please make a PR ๐Ÿ˜Š