Optional moment.js Part 2

[prayerslayer]

Basically it's #1046 minus moment in tests.

The idea is that after this PR I can:

1. create a new date_utils without moment
2. copy tests, give components a flag which utils to use and switch the utility module in test code
3. refactor react-datepicker to work with whatever while having a good idea if I broke something

[codecov]

Codecov Report

Merging #1052 into master will increase coverage by 0.4%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #1052     +/-   ##
=========================================
+ Coverage    83.1%   83.51%   +0.4%     
=========================================
  Files          13       13             
  Lines         817      837     +20     
  Branches      141      141             
=========================================
+ Hits          679      699     +20     
  Misses         33       33             
  Partials      105      105

Impacted Files	Coverage Δ
src/date_utils.js	92.44% <100%> (+0.99%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3c9d89...7b82f65. Read the comment docs.

[aij]

Looks good to me. @martijnrusschen Care to take a look?

[aij]

It might be a bit cleaner to use separate functions to parse a date and to get the current date. We can leave that for later though, since moment() did serve both purposes too.

[prayerslayer]

Yeah, had exactly the same thought :)

[martijnrusschen]

Seems solid. Lots of changes, so lets hope we don't break anything. How should we move forward with this. First merge part 1?

[prayerslayer]

How should we move forward with this. First merge part 1?

Since this part contains also part 1, we can close the other and merge only part 2.

[aij]

@martijnrusschen Sounds good, that way it's clear the changes to the code are independent from the changes to the unit tests.

[martijnrusschen]

@prayerslayer Part 1 got merged. Can you rebase your PR so that we can merge part 2 as well?

[prayerslayer]

Done.

[prayerslayer]

WIP of Part 3 is here: https://github.com/prayerslayer/react-datepicker/tree/feature/optional-moment-part3

• I'll probably need to refactor how locales are handled. A major advantage of date-fns is that it doesn't include a bazillion locales right away, so we'd probably need to give up string locales (locale="fr") in favour of locale data that is useful for the formatting function (locale={require("date-fns/locale/fr")} / locale={moment.localeData('fr')}).
• What gives me a bit of headache is the UTC offset. As far as I see there is no way to get a JS Date with a timezone other than what you're currently in. Ofc you can do something like new Date(Date.UTC(year, month, day)), with year, month, day being values in UTC, but the Date object will still show GMT+2 or whatever. Not sure yet how to work around this, advice is welcome.

[martijnrusschen]

I'll probably need to refactor how locales are handled. A major advantage of date-fns is that it doesn't include a bazillion locales right away, so we'd probably need to give up string locales (locale="fr") in favour of locale data that is useful for the formatting function (locale={require("date-fns/locale/fr")} / locale={moment.localeData('fr')}).

Sounds fine. As long as we discuss this as a breaking change

What gives me a bit of headache is the UTC offset. As far as I see there is no way to get a JS Date with a timezone other than what you're currently in. Ofc you can do something like new Date(Date.UTC(year, month, day)), with year, month, day being values in UTC, but the Date object will still show GMT+2 or whatever. Not sure yet how to work around this, advice is welcome.

Not sure how what's desired here. Any suggestions?

[prayerslayer]

Hi, thanks for taking the time. I already opened a PR to discuss details about this. #1059 It has a proof of concept too.

Not sure how what's desired here. Any suggestions?

Yeah. My proposal is that all timezone related functions would be no-ops when using date-fns, until the library can work with TZs (if ever). That way

• we don't build a subset of moment with less tests
• we don't extend the underlying library in a non-obvious way
• users still get to choose based on their needs: Want a really small bundle and TZs are not an issue? Go with date-fns-backed datepicker. Definitely need TZs? Well, then you're stuck with moment anyways.

At first I tried to extract the TZ logic from moment into something separate, but quickly realized I'm building moment on top of date-fns, as you have to consider TZ information for all regular operations like adding hours etc. That's an option too, but it can be done later imo.