moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.32k stars 730 forks source link

Islamic Calendar 1 day ahead #1540

Closed btxtiger closed 10 months ago

btxtiger commented 10 months ago

Describe the bug Seems the islamic calendar is 1 day ahead.

To Reproduce

let date2 = DateTime.fromISO('2023-11-14').reconfigure({outputCalendar: "islamic"})
console.warn(date2.toLocaleString(DateTime.DATE_FULL));
// 1. Dschumada I 1445 AH
// expected output for `islamic`: 30. Rabiʻ II 1445 AH

Actual vs Expected behavior According to this site (and others): https://datehijri.com/en/ 1st Jumada should be on the 15th November and not on 14th November.

Both islamic calendar types islamic and islamicc deliver the same.

Desktop (please complete the following information):

icambron commented 10 months ago

This seems to be an issue with the Intl API. You can test it directly:

new Intl.DateTimeFormat("de-u-ca-islamic", { year: "numeric", month: "long", day: "numeric" }).format(new Date(2023, 10, 14))
// => '1. Dschumada I 1445 AH'

I tried it in Chrome, Firefox, and Node on my Mac; they all behaved as above. Ultimately, that code is what Luxon is doing under the covers, so it gets that result too. So you may very well be right that it's incorrect, but unfortunately there's not much Luxon itself can do about it. I recommend raising the issue upstream with browser folks.

btxtiger commented 10 months ago

@icambron Thanks for the explanation and fast response. This way I could resolve the issue: For my knowledge, the most commonly used islamic calender is islamic-umalqura. So with the adjusted calendar, I am getting the expected date:

const b = new Intl.DateTimeFormat("de-u-ca-islamic-umalqura", { year: "numeric", month: "long", day: "numeric" }).format(new Date(2023, 10, 14))
console.warn('islamic-umalqura', b);
// 30. Rabiʻ II 1445 AH

It think it should be really considerable to add it to the library, or make it possible to pass the original calender names. A quick test using

const islamic = date.reconfigure({ outputCalendar: 'islamic-umalqura' });
console.warn('islamic', islamic.toLocaleString(DateTime.DATE_FULL));

was not successful.

icambron commented 10 months ago

@btxtiger Luxon doesn't have any notion of the different output calendars; it just passes them to Intl, so it's hard to imagine that not working the same way. This works for me:

DateTime.fromISO('2023-11-14T00:00').reconfigure({outputCalendar: "islamic-umalqura", locale: "de"}).toLocaleString(DateTime.DATE_FULL)                                                                                                                                                                                                       
//=> '30. Rabiʻ II 1445 AH'

(the locale thing is just me trying to match what I assume your default locale is)

btxtiger commented 10 months ago

@icambron yes, you are right! It was a mistake in my code. Then we can probably just add islamic-umalqura to the list in the docs as example. I think it it would be helpful for others since it can also be found by Google. There are a lot of buggy custom implementations available for islamic/hijri calendars, which are unnecessary if people would know about the Intl API.

icambron commented 10 months ago

@btxtiger want to write a PR for that? The file is here