KosherZmanim icon indicating copy to clipboard operation
KosherZmanim copied to clipboard

Replace Luxon with Temporal

Open mjradwin opened this issue 9 months ago • 8 comments

Here's an implementation of #28, which replaces the use of Luxon with Temporal.

The transformation was pretty straightforward. Mostly occurrences of DateTime become Temporal.ZonedDateTime, however there are some places where we use Temporal.PlainDate.

The other tricky part is comparisons, which require use of a special static function Temporal.ZonedDateTime.compare() instead of just using < or > or <=.

I've added an additional unit test for the NOAA calculator with an elevation, which lines up to the Java implementation for a handful of functions down to the second.

mjradwin avatar Mar 31 '25 02:03 mjradwin

Would you be willing to review this pull request?

mjradwin avatar Apr 08 '25 03:04 mjradwin

@mjradwin I will try to take a look over Yom Tov. However, I think this should wait to be released until Temporal has higher usage.

BehindTheMath avatar Apr 09 '25 16:04 BehindTheMath

https://developer.mozilla.org/en-US/blog/javascript-temporal-is-coming/

ShlomoCode avatar Apr 17 '25 16:04 ShlomoCode

Not quite there yet. See https://caniuse.com/temporal

KosherJava avatar Apr 17 '25 16:04 KosherJava

@mjradwin I will try to take a look over Yom Tov. However, I think this should wait to be released until Temporal has higher usage.

Thanks for reviewing. If you want to wait for the Temporal spec to be fully ratified (it's currently TC39 Stage 3, and not all the way to Stage 4), I do understand. How would you feel if release my fork on npm as @hebcal/zmanim? I'd like to begin using the software on the hebcal.com website - there is at least one feature request that could be satisfied by having this available

https://hebcal.userecho.com/communities/1/topics/1552-please-add-zmanim-according-to-the-baal-hatanya

mjradwin avatar Apr 17 '25 18:04 mjradwin

@mjradwin You can install with npm directly from your fork

npm i https://github.com/mjradwin/KosherZmanim/tree/master

ShlomoCode avatar Apr 17 '25 18:04 ShlomoCode

Hi @BehindTheMath, we'd love to get this merged into your tree if you're willing to review the PR.

I understand the reluctance to release. It's just one data point, but we've been using the temporal-polyfill package in production for over a year now and are very comfortable using this as a dependency.

We'd rather not wait for Temporal to be widely adopted as a browser and node.js builtin as this is probably still a year or two away. Are you OK with us releasing a @hebcal/KosherZmanim fork in the interim?

mjradwin avatar May 22 '25 05:05 mjradwin

Hi @BehindTheMath, would you be willing to merge this into your tree? I've got at least one request that would benefit from this update

https://hebcal.userecho.com/communities/1/topics/1552-please-add-zmanim-according-to-the-baal-hatanya

mjradwin avatar Jul 29 '25 17:07 mjradwin