jalaali / moment-jalaali

A Jalaali (Jalali, Persian, Khorshidi, Shamsi) calendar system plugin for moment.js.
MIT License
940 stars 162 forks source link

fix: localData is not a function #226

Closed nabc closed 2 years ago

nabc commented 2 years ago

Hi First I want to thank you for this amazing library, which makes us developers lives easier. I've encountered the error moment.localeData is not a function and used the solution that was provided in #207 and it worked. The problem with the solution is that I have to modify this library's index file in node_modules which is not the proper way to handle the error. So I created this pull request and applied the solution. I hope to see it being merged pretty soon. Thanks

behrang commented 2 years ago

Hi, Thanks for the PR. Are you sure that this is not going to break old code?

Maybe this fixes usage in Vue.js but breaks older usage in Node.js.

@alitaheri Do you have any opinion on this?

nabc commented 2 years ago

Actually, I'm using Material-ui picker library in a React project which has date-io library as dependency, which in turn has moment-jalaali library as dependency, it is 0.9.1 version if I'm not mistaken. I have opted to use Vite js instead of CRA which resulted in the error. That being said, I don't think it will break older usages. When requireing a package in a js file, it will point to the main field in package.json, which includes name of the main file of the package and its relative address to package.json file. But it is possible to be extra specific about the file you wish to include and add the file name after package name, it's a bit overuse when package exports only one file, but useful when a package has multiple files with different functionalities to offer. So using require('moment/moment') is more detailed way of using require('moment'), it should not break older usages.

behrang commented 2 years ago

Great, thanks. v0.9.4 is published.

nabc commented 2 years ago

Thanks