deseretdigital / dayzed

Primitives to build simple, flexible, WAI-ARIA compliant React date-picker components.
https://dayzed.netlify.app
MIT License
663 stars 27 forks source link

Replacing some utils for date-fns functions #17

Closed arthurdenner closed 6 years ago

arthurdenner commented 6 years ago

As suggested on #10 and #14, this PR aims to replace some functions on the utils module for date-fns equivalents. The following were replaced:

mkartchner994 commented 6 years ago

This is looking great. Thanks for the work! Should we also look at using https://date-fns.org/v1.29.0/docs/differenceInMonths to replace the monthDiff function in utils?

arthurdenner commented 6 years ago

Thanks, @mkartchner994.

I didn't replace them because they return different values, probably because differenceInMonths returns the number of full months between the given dates while monthDiff returns the difference between the .getMonth() calls. E.g:

dateFns.differenceInMonths(new Date(2014, 8, 1), new Date(2014, 0, 31)) // returns 7
// the functions have different signatures, but that's not a problem
monthDiff(new Date(2014, 0, 31), new Date(2014, 8, 1)) // returns 8
mkartchner994 commented 6 years ago

@arthurdenner Ah, I see. Is that the same for the differenceInCalendarMonths method? https://date-fns.org/v1.29.0/docs/differenceInCalendarMonths

arthurdenner commented 6 years ago

I didn't see this function! And no, differenceInCalendarMonths returns 8 too. I'll commit this change.

mkartchner994 commented 6 years ago

@arthurdenner This looks great. Thanks!

I think I have decided after all that date-fns should be a peer-dependency and not get bundled into the final dist output. Because of this, what I am planning on doing is bumping up to the next major version number to v2 so this won't cause anyone's v1 to break if they don't have date-fns brought into their project.

I will do some work on that tonight or tomorrow and then let you know when I cut that release.

Also, I would like recognize your work based on the "All Contributors" specification - https://github.com/kentcdodds/all-contributors. If that is alright with you, another pull request with that would be great - should just be able to run the add-contributor script in package.json. Or I can commit that on my end as well. Just let me know.

Thanks again!

arthurdenner commented 6 years ago

Thanks, @mkartchner994. :heart:

Feel free to add me on the contributors list, I tried to run add-contributor but got some error related to the CLI that I couldn't solve :sweat_smile:

I closed #14 and you probably want to close #10. I'll open a PR to solve #5 if that's okay.

mkartchner994 commented 6 years ago

@arthurdenner v2.0.1 has bee published. Thanks for your work on this. I have also added you to the contributors list 👍