wanasit / chrono

A natural language date parser in Javascript
MIT License
4.61k stars 339 forks source link

Expose individual locales #509

Closed jdicarlo-dh closed 1 year ago

jdicarlo-dh commented 1 year ago

Hello! This PR is to allow importing individual locales from Chrono without all of them being imported implicitly.

What changed

Why?

We're using Chrono with tools that utilize a single static binary for Node.js. These binaries are built with the --without-intl flag to reduce their size, which is fine for our use case as we don't need to support languages besides English.

However, when using the chrono library, other locales such as ru are loaded implicitly which causes an error due to the intl module not being present:

Invalid regular expression: /* omitted */: Invalid property name in character class

Other Notes

These changes do not break the library's API and none of the tests required any modification. There were 5 failures when I ran tests locally, but I also got these errors when running tests on the master branch, so I don't think they're related to my changes. I have also updated the README.md with information about using the individual locales

Please let me know if I've missed anything or if there are any other questions/comments/concerns, and thank you for such a useful library! 😄 ❤️

wanasit commented 1 year ago

Hello @jdicarlo-dh. Thanks for working on this.

I agree with your idea, but because your change is big, I'll need to take some time to review and try it. It'd be nice if you could resolve the conflict with src/index.ts too.

jdicarlo-dh commented 1 year ago

No worries, take your time! I will gladly fix up the conflict in the meantime

jdicarlo-dh commented 1 year ago

@wanasit I rebased my branch with master and made some changes to integrate those changes appropriately with mine. I also updated the exports on the locale indexes to match what's exported by the root index.

@JoakimNil would love to get your eyes on this as well!

jdicarlo-dh commented 1 year ago

Hi @wanasit just wondering if you've had a chance to take a look at this yet. I've rebased it with the most recent changes from master so it should be good to go for testing 😄

wanasit commented 1 year ago

Hello @jdicarlo-dh.

Could you look at my comments and make some naming changes? Overall, the change looks good!

Also, if you know which RegEx in ru cause the problem, could you open a separate issue for that?

jdicarlo-dh commented 1 year ago

@wanasit I changed the name from parsing.ts to types.ts and replied to your other comments 😄

coveralls commented 1 year ago

Coverage Status

Coverage: 91.139% (+0.09%) from 91.047% when pulling 74187222a2b7b7dcf64d0fd4cde5a9ceb9b38044 on jdicarlo-dh:expose-individual-locales into bd838bd89aa05f3c34489794dab29c184538aa14 on wanasit:master.

wanasit commented 1 year ago

I apologize that I forgot to publish the new version for months. This change has been included in v2.6.4.