Use Moment JS for dates

[peacechen]

CalendarPicker is transitioning to Moment JS, a more robust and convenient date library than the stock Javascript Date object. This library will continue to support JS Date props. The only change needed by the parent project is to add moment to the dependencies in package.json. For example:

  "dependencies": {
    "moment": "^2.18.1",
  },

[8enmann]

i followed the readme and got an error on first run that moment wasn't found. add to docs?

[peacechen]

Moment is set as a peer dependency which is the preferred way to specify shared dependencies. https://github.com/stephy/CalendarPicker/blob/master/package.json#L17

It should print a message after npm install that Moment JS is required as a peer dependency.

Also, specifying a specific version (or range) would make this project brittle as far as conflicting versions used by various apps.

[8enmann]

it did, but with react native there are a ton of similar looking messages that don't cause errors. adding to docs would make it easier.

[rolfb]

Moment isn't particularly performant and has some bad behaviour like it modifies the self which can cause unexpected bugs. Would suggest using date-fns instead.

Example which would be unexpected for me:

const today = moment(new Date)
const tomorrow = today.add(1, "day")

// now the const today is actually set to the day after

[peacechen]

Agree that updating the documentation is helpful. I'll merge a PR immediately.

Moment has its quirks (such as add() mutating in-place), but there are idiosyncrasies with any library. Do you have benchmarks that indicate Moment is a performance drag? I've not found it to be a problem. The code snippet above could be optimized by eliminating the redundant new Date constructor.

const today = moment();

[rolfb]

Let me know if you can find any idiosyncrasies with date-fns.

Looking around the interwebs, moment is only faster at formatting dates compared to date-fns. If I remember correctly it's hard to import just the modules you need with moment - you will always get all of it.

Here's one performance test https://raygun.com/blog/moment-js-vs-date-fns/ - moment really starts taking a toll when there's a lot of dates (like calendars).

[peacechen]

The happy-path cases are easy, it's the corner cases that make things difficult. This library initially rolled its own date manipulation functions but suffered from a number of bugs related to day boundaries, time zones, leap years, etc. Moment JS is a well tested library that takes care of all those pesky bugs. CalendarPicker doesn't make so many Moment JS calls to significantly impact performance. Are you seeing performance issues?

We could refactor to use date-fns, but there are people using this in production who expect Moment objects returned by CalendarPicker. Is there a way to use date-fns but return Moment-compatible objects? (without actually using Moment)

[rolfb]

If people are locked-in to moment as an expectation then I won’t touch this. Could use moment just at the boundaries and date-fns on the internal side, but probably more hassle than it’s worth just now.