coachcare / npm-datepicker

Material Datepicker Fork with TimePicker by CoachCare
MIT License
32 stars 18 forks source link

Get rid of moment #35

Open Enngage opened 5 years ago

Enngage commented 5 years ago

I have to stop using this library because of the fact it depends on Moment.js. Moment.js is a good library, but its incredibly bulky and easily adds 1MB to angular bundle size which is just unacceptable. Team behind moment is currently working on Luxon which is much better alternative.

matheo commented 5 years ago

Good point, I will check Luxon to see how hard is to get an Adapter provider ;)

Enngage commented 5 years ago

@matheo that should be fairly easy. I've created my own luxon adapter. Its not fully tested or anything, but works fine for us. I'll probably roll it out to npm & git (so others can fix potential issues) and let you know.

matheo commented 5 years ago

@Enngage please, just upload it to a repo of yours and I will check it ;)

ghost commented 5 years ago

Day.Js is a nice alternative to me

⏰ Day.js 2KB immutable date library alternative to Moment.js with the same modern API

https://github.com/iamkun/dayjs

mtraynham commented 4 years ago

Trying this library out. I too am looking to avoid moment, but I can suffice it for now. I did notice however that the native variant provided is broken. There's a null pointer when retrieving date.getFullYear(). This happens because the library does a diff of the current date with the new date, but the current date is undefined.

matheo commented 4 years ago

@mtraynham do you mean that the MatMomentDateModule provided by @coachcare/datepicker doesn't work? or you're importing it from @angular/material?

mtraynham commented 4 years ago

Hi @matheo , no the MatNativeDateModule provided by @coachcare/datepicker doesn't work.

I'll walk you through it.

  1. Setting the initial activeDate in calendar.ts performs a dateAdapter.compareDate with the oldActiveDate and this._clampedActiveDate, where oldActiveDate would be undefined.
  2. NativeDateAdapter doesn't override the base DateAdapter implementation for compareDate, but it does call this.getYear(first), where first would be undefined.
  3. NativeDateAdapter just calls date.getFullYear(), but date there is undefined thus throwing an exception.

The moment adapter seems to work without issue because it clones the date before it retrieves the year and the clone implementation returns a moment() of today where it may have gotten an undefined value.

Honestly, I would have preferred using the MatNativeDateModule, but because the entire source tree is condensed into a single distributed file, you can't get around the import moment from 'moment-timezone' that's included in the compiled JS. So I installed moment and went with the MatMomentDateModule...

matheo commented 4 years ago

@mtraynham all right, I just published 1.0.1 avoiding that undefined. I cannot push to this repo but it's up: https://www.npmjs.com/package/@coachcare/datepicker

BTW, moment is not bundled with the lib but required as external dependency, if it's not in your project the MomentAdapter won't work but it will be fine if you don't use it. The only warning is about the peerDependencies, but that's not mandatory.

In the other hand, which Angular version are you using?

mtraynham commented 4 years ago

@matheo Angular 8.2.7

BTW, moment is not bundled with the lib but required as external dependency, if it's not in your project the MomentAdapter won't work but it will be fine if you don't use it. The only warning is about the peerDependencies, but that's not mandatory.

I'll have to try that out after the fix. I believe it was a problem because even though I'm not actually using moment, the Angular build was pulling in node_modules/@coachcare/datepicker/fesm/coachcare_datepicker.js which is a single file bundle of all the code and at the top of that file, it has:

import * as momentNs from 'moment-timezone';

I'll have to investigate why Angular/Webpack didn't instead pull in the esm build which is not bundled, but rather the compiled modules split out.

matheo commented 3 years ago

Hi guys! I've moved the source code to https://www.npmjs.com/package/@matheo/datepicker and only provided the NativeDatePicker with native Date instances