commenthol / date-holidays-parser

parser for worldwide holidays
ISC License
46 stars 22 forks source link

move moment dependencies to date-fns-tz #53

Open mcshaz opened 1 year ago

mcshaz commented 1 year ago

I wanted to move the library away frommoment and moment based tools in order to decrease bundle size. There are likely significant improvements in processing speed as well.

All tests pass, although I have not worked on documentation as yet pending your thoughts.

If using date-fns-tz (date-functions-timezone), the native Intl API is used with hopefully significant decrease in bundle size. It would have to be polyfilled in browsers without support - as IE11 is now unsupported by Microsoft for over 6 months (and has current usage < 0.6%), this should have little impact. Android will be affected and a good polyfill (which I will including in the documentation if you were interested in merging these changes) is maintained by Format.JS.

The hijri-calendar folder uses a small method in the script to create the hijri-calendar JavaScript object. This will not polyfill well as polyfills only tend to use the gregorian calendar. This should not be a problem as this script it is never run client side.

The new script differed slightly on a few dates produced in - for example the hijri date "3/1/1389 AH" (M/D/YYYY) had corresponded to the 17th May 1969 with the moment-hijri library but is now the 18th May 1969. I have tested these dates on Node and several major browsers and they seem to agree with the new version (18th May 1969). Given the effort ICU JIRA (the engine backing Intl API in most modern browsers) have gone to working with the Saudi ministry, I would suspect the newer implementation produces the more correct result, but I am not able to read Arabic and the canononical source is http://www.ummulqura.org.sa/

Appologies about the multiple commits - these should be squashed.

F1nnM commented 1 year ago

I would like to bump this, is there any possibility that this transition will happen anytime soon?

mcshaz commented 1 year ago

@F1nnM - thanks for your interest. The changes are all working when the 3 libraries (date-holidays, date-holidays-parser and caldate) are used together - I can send you the setup as workspaces so you can build distributable files. It is under half the size and all tests pass (plus a few additional tests). I believe @commenthol is wanting to compare to another pull request to using Luxon instead of date-fns-tz. Ultimately given the inability to tree shake Luxon and the relatively few functions required, date-fns-tz will be a smaller bundle.

I added a few opinionated changes I would also like to hear back about from @commenthol. Primarily this involves moving caldate to a few named exports rather than a single default export. This has the advantage that a) number of utility functions for which code is duplicated in date-holidays-parser have a single source. b) allows an architecture whereby minimal change is required in years to come when the ECMAScript Temporal spec has been rolled out (in consuming this new spec and also removing dependency on any timezone library).

F1nnM commented 1 year ago

Oh, that would be great. Can you attach them here or send them to me under ######?

mcshaz commented 1 year ago

@F1nnM I have put the workspace structure under a repo at https://github.com/mcshaz/date-holidays-workspace/ with the readme giving instructions on which branch to install in which folder

F1nnM commented 1 year ago

It took some tweaking and hacky workarounds, but I got it to work in the end. Thank you!