stephy / CalendarPicker

CalendarPicker Component for React Native
802 stars 371 forks source link

Issue with date-fns imports (_____ is not a function) #367

Open owattenmaker opened 7 months ago

owattenmaker commented 7 months ago

Just trying out this library. I noticed you've just switched over to using date-fns which is great since I already use this in my project.

On install and open however I get errors with the way you import functions from date-fns.

for instance you have:

import { getMonth } from "date-fns/getMonth"

this fails for me. However either

import getMonth from "date-fns/getMonth"

or

import { getMonth } from "date-fns"

works. Each of the individual files from date-fns just have default exports, and no named export. Or the index file has the named export. Unsure of which version you're using of date-fns, this is for version 2.30.0 for me locally.

Anyone else? solutions? I will use this library with a patch package to fix the imports so they work for me, but should be a simple fix for everyone else.

peacechen commented 7 months ago

@tranjog updated this library to use date-fns. Did date-fns change their exports between versions?

owattenmaker commented 7 months ago

I do see some breaking changes from v2 to v3 mentioning imports. https://date-fns.org/v3.3.1/docs/Change-Log#v3.0.0-2023-12-18

So that must be it.

You may wish to add a peer dependency of v3 or higher.

Additionally, I did try upgrading to version 3 of date-fns, but it appears that date-fns-tz library is not yet updated to work with v3, so I cannot do this. Perhaps you could just switch to using the import { name } from "date-fns" format which works with both v2 and v3.

peacechen commented 7 months ago

That's it. They really shouldn't have broken imports like that. It would have been better for them to export both the default and named versions.

The peer deps should have this set to >= 3.0 to inform users that's needed.

tranjog commented 7 months ago

Comparing the date-fns source files, v3 does named exports only: addDays v3

and v2 does default exports only: addDays v2

However, both support import {addDays} from 'date-fns'

So it'll be trade-off between supporting both v2 and v3 and giving up (potential) tree-shaking benefits by importing from date-fns/addDays, or only supporting >= 3.0.

Any opinions on this? I'm leaning towards the former.

peacechen commented 7 months ago

Supporting older versions isn't a good use of resources IMHO. It may be useful temporarily, but long term it's throw-away work. It's not a heavy burden for consumers of this library to upgrade to date-fns 3.x.

Another option is to create a beta release branch that modifies the imports to support 2.x. That would be a dead-end branch and not maintained unless someone who needs it contributes to doing the work.

tranjog commented 7 months ago

Okay, thanks! I'll update the peer dependency only, the README already states that we require date-fns >=3.0.

What about: "Whilst we do not officially support date-fns 2.x, you can apply this patch file at your own risk" and link to this thread?

Would you be able to upload your patch @owattenmaker ?

peacechen commented 7 months ago

A patch is a last resort option when the maintainers aren't available. Since we're here to publish new versions, it would be best to commit the changes to a branch and publish an alpha version that doesn't get pulled by npm unless specifically requested.

I've made a branch datefns2 for this. @owattenmaker please create an MR that targets this branch.

DFRZ7 commented 4 months ago

Hi, just wanted to mention that the I had to create a patch as well in order to move forward on this.

Changing from:

-import { getMonth } from 'date-fns/getMonth'; -import { getYear } from 'date-fns/getYear'; -import { isSameDay } from 'date-fns/isSameDay'; -import { isSameMonth } from 'date-fns/isSameMonth';

to:

+import getMonth from 'date-fns/getMonth'; +import getYear from 'date-fns/getYear'; +import isSameDay from 'date-fns/isSameDay'; +import isSameMonth from 'date-fns/isSameMonth';

Helped us out.

Will keep an eye on this.

Thank you.

AleksandrTermenzhy commented 1 month ago

Hey @peacechen,

I wonder, given this incompatibility with the pre-3 versions of the date-fns, is there any reason for not updating peerDependancies of the library to something more specific:

"peerDependencies": {
    "date-fns": "^3.0.0"
}

As now it causes issues in a monorepo setup when other libraries define lower date-fns version.

peacechen commented 1 month ago

@AleksandrTermenzhy Yes that would help to avoid/notify other libraries that are using lower versions. Would you mind submitting a PR that changes both the peer dependency and dev dependency version to >= 3.0.0 ?