search

owenmead / Pikaday

A refreshing JavaScript Datepicker — lightweight, no dependencies, modular CSS
Other
153 stars 73 forks source link

moment shouldn't be a dependency #60

Open ruffle1986 opened 6 years ago

ruffle1986 commented 6 years ago

Hi,

Before I start, let me thank you for pikaday-time. It's a great library and it saved us a lot of time. So thank you for your hard work and making it happen. 🍻

I know you no longer maintain this repo but I'm still hoping that we can solve my problem somehow.

I've noticed that moment.js is just an optional dependency of the original Pikaday lib and I'm not sure if that's the case with your fork. I've noticed you marked moment as an optionalDependency in your package.json so the answer is yes I guess.

We use pikaday-time in our application and npm installs moment every time because it's a dependency of your lib. We're building an angular app and we try to get rid of moment.js and switch to another date/time manipulation library which has a smaller footprint in the final bundle (e.g date-fns). But since moment.js is a dependency hence npm installs it every time and Pikaday has a dynamic module loading strategy, moment will be part of the final bundle however we have eliminated everywhere in our source code.

I think if we set moment as a peerDependency that would solve my problem.

I'd be more than happy to open a PR with this patch if you helped me with the release.

What do you think?

ruffle1986 commented 6 years ago

I had a little workaround with that and I've realised that the optionalDependecies may have used wrongly here. Correct me if I'm wrong but according to the npm docs, you should use it if you don't want npm to fail if a dependency is not found in the given registry:

https://docs.npmjs.com/files/package.json#optionaldependencies

If a dependency can be used, but you would like npm to proceed if it cannot be found or fails to install, then you may put it in the optionalDependencies object.