Update DatePicker so it doesn't require a moment object

[jrios]

Forcing consumers of the DatePicker to pass a moment object in is a bit heavy-handed and offers unnecessary insight into the implementation of the DatePicker component. I think it would be beneficial to allow the DatePicker component to take in a Date or string and manage the conversions internally.

Thoughts?

[Darrken]

I like this idea. It would simplify things a lot for the consumer.

[aabenoja]

I think it'll be fine to open it to a moment object, Date and string. From an ease of use perspective we're probably already using moment downstream, so having to convert down to a string or date object only to have the DatePicker convert it back is a bit redundant. I am not 100% certain, but if we pass a moment into moment the returned object shouldn't be any different.

[jrios]

Sure, I guess we can have all three. To be honest, I don't love that moment is a dependency at all. I would love to see about forking the react-datepicker and removing the moment dependency. It is such a large library that ends up in upstream bundles.

[aabenoja]

I like that plan better, honestly. I get that moment makes manipulating dates easier so it won't be going away from react-datepicker anytime soon. It sounds like if we follow through all we need is a suitable replacement for moment to make date manipulation easier. A couple of alternatives is dayjs, luxon and date-fns.

[jrios]

Yeah, I don't think we'd get them to accept a PR removing moment. but there's no reason we cannot push our own fork to npm.

[Darrken]

I'm looking into replacing the moment dependency in a fork of react-datepicker and have found 2 good potential replacements: date-fns (which I know Jake likes) and dayjs. The downside of date-fns is it has no support for UTC time and only supports native js Date. I assume we need to store UTC time when we save dates since we deal with people across all US timezones. Dayjs has a very similar API to moment and is only 2k. It is lacking some of the functions moment and date-fns have.

Maybe a mix of both? Thoughts?

One of the tricky bits of the datepicker fork is supporting the current localization support. Dayjs has a localization plugin but it isn't as robust as moment's so there's some challenge there trying to accomplish the same thing with different functions.

[jrios]

Check out this PR to date-fns. It hasn't been merged yet, but looks close to being merged and introducing UTC functionality. I have not looked at DayJS before, but 2K is definitely less heavyweight.

[jrios]

Another thing to consider - how important is time when using the date picker? I can imagine it might be important down the road for other components, but for picking a date, is the time necessary?

[aabenoja]

I think this is a good idea. I'm actually having trouble coming up with a reason to bubble up time when using date picker. This will keep our API pretty light. If for some reason it becomes important to capture the time or timezone it would be better to leave that as a concern for the application.

[Darrken]

That would significantly simplify it. I wonder if we should look into another control entirely as this has so much baked in.

[aabenoja]

It might be even more of a lift to completely switch out react-datepicker for something else as opposed to forking and replacing moment. While the latter has a larger development effort switching to another datepicker library will require another accessibility audit.

[aabenoja]

If I'm reading Hacker0x01/react-datepicker#1349 correctly no one is opposed to ripping out moment on that end. I say we try our hand at making a PR to react-datepicker.

EDIT Hacker0x01/react-datepicker#382 is a more comprehensive look at progress for ripping out moment. Most of the work done is around consolidating usage to a single file.

[Darrken]

I'm about 95% there, working on a fork and replacing moment.js with dayjs and a couple of date-fns functions. I went with dayjs primarily because it has a UTC plugin and localization functions similar to moments. Unfortunately I've discovered today that the UTC plugin has bugs in it (which is why I've been stumped on tests for a day or two longer than I thought I'd be).

I'd prefer to just use date-fns at this point, and wonder if I can hack together the couple of UTC methods (or remove them and let the client sort it out?). That PR you linked to did a good job abstracting the moment.js functions to one file so it's not too hard to swap them out. It's just figuring out the intent of all the various utility functions and making sure they still work the same when underlying dependency functions are slightly different.

[aabenoja]

On one hand I don't think anyone needs the time functionality. One of the comments I saw went as far as to say that the component only needs to track the month, day and year as integers in its state. While I would like to rip out the time functionality I do not know how much of a lift that would be and whether or not we should put off that aspect for a followup PR. Either way, ripping out moment will save us 200kb.

[Darrken]

Yeah I'm debating whether to rip out time altogether. It would certainly be easier to deal with without the time component, but I'm pretty sure that would cause my fork to be a standalone project at that point. We'd miss out on future main branch enhancements unless we pulled them in manually, which I would not enjoy doing.

[aabenoja]

Since finding those two issues on the react-datepicker repo I would rather all of this be a PR towards the main project rather than a fork for our purposes. If anything we can either propose the time-less changes as part of this date-fns PR or extracted into a second PR for the rest of the maintainers to weigh in on.

[Darrken]

My PR :)

[aabenoja]

Now that the PR has been merged and [email protected] has been released did we want to resolve this ticket with #229 or in a separate PR

[jrios]

Huge that this finally landed. I personally think pushing it into the next major set of changes with #229 makes the most sense.

[jrios]

closed with #231