stephy / CalendarPicker

CalendarPicker Component for React Native
807 stars 372 forks source link

Replace moment with date-fns #359

Closed tranjog closed 10 months ago

tranjog commented 10 months ago

I think there are good reasons to replace moment.js with date-fns. A way lighter, more performant, more popular, library, without the mutability quirks of moment.

This PR removes all imports of, references to, and moment specific methods from the code base, and replaces it with equivalent utilities from date-fns - which is added as a dependency.

It should be fully backwards compatible with earlier versions.

I've updated the version to 8.0.0 and reflected the changes in the README file.

Issue #360

peacechen commented 10 months ago

Thanks @tranjog Looking forward to this getting tested and released. The major rev should be bumped to 8.x so that it doesn't break users who expect it to use Moment.

Please also update the Readme to note the migration from Moment to Date-fns

tranjog commented 10 months ago

Thanks for your prompt reply @peacechen ! I've opened the PR for review, and will be happy to implement any feedback.

peacechen commented 10 months ago

Thanks @tranjog

Unfortunately I don't have time to test these changes. Please update and test the sample app to ensure it's operating properly. https://github.com/stephy/CalendarPicker?tab=readme-ov-file#sample-application

What do you think about releasing this as 8.0.0-alpha so that you can start using it? That lets people know that it's not fully tested yet.

tranjog commented 10 months ago

Thanks @tranjog

Unfortunately I don't have time to test these changes. Please update and test the sample app to ensure it's operating properly. https://github.com/stephy/CalendarPicker?tab=readme-ov-file#sample-application

What do you think about releasing this as 8.0.0-alpha so that you can start using it? That lets people know that it's not fully tested yet.

@peacechen I've changed the version to "8.0.0-alpha" for now, and hope to get some testing and feedback from the community so we can make it 8.0.0 soon.

I've already updated the sample app, however - I had to upgrade the expo, react and react-native dependencies to get it working locally, and I've now pushed these changes too as it will be easier for a new developer to follow the instructions in the README and get the sample app up and running.

Here's a screenshot of the sample app on the latest commit in this PR - installed by following the steps in the README: Screenshot 2024-01-09 at 10 15 16

peacechen commented 10 months ago

Thanks for continuing to work on this. I'll merge and publish shortly. Would you like to be added as a collaborator? I can add you to the npm project for publishing new versions. We'll need @stephy to add you as a collaborator to this repo as she is the owner.

Regarding date-fns imports, does it support individual imports?

import differenceInDays from 'date-fns/differenceInDays';

instead of

import { differenceInDays } from 'date-fns';

The former reduces import size when the transpiler doesn't support tree shaking.

tranjog commented 10 months ago

Thanks @stephy, I'd be happy to be added as a collaborator.

Yes, you can do individual imports:

import addMonths from 'date-fns/addMonths'
import subDays from 'date-fns/subDays'

I'm not sure if it's necessary to individually import the utils to benefit from tree shaking.

With the function-per-file style, you can pick just what you need and stop bloating your project with useless functionality. It works well with modern module bundlers such as webpack, Browserify, and Rollup and also supports tree-shaking. date-fns.

peacechen commented 10 months ago

That description reiterates the point about import size: "With the function-per-file style, you can pick just what you need and stop bloating your project with useless functionality."

It's best to import each function from their individual files:

import addMonths from 'date-fns/addMonths'

@stephy has been inactive unfortunately so I don't know when she'll return to add you to the repo.