moment / luxon

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

Incorrect weekNumber on Sundays #1587

Closed weklund closed 5 months ago

weklund commented 5 months ago

Describe the bug I'm writing test cases to ensure given a particular DateTime, Luxon is giving me the uncorrect weekNumber to support an Alfred<->Obsidian plugin I'm working on.

export function getWeekNumber(date: DateTime): number {
    return date.weekNumber
}

describe("getWeekNumber", () => {
        Settings.defaultLocale = "en-US"
        Settings.defaultWeekSettings = {
            firstDay: 7,
            minimalDays: 1,
            weekend: [6, 7]
        }

        const opts = { locale: "en-US", outputCalendar: "gregory" } as LocaleOptions

        const cases = [
            [DateTime.local(2024, 1, 1, opts),    "Monday",  1],
            [DateTime.local(2024, 1, 7, opts),    "Sunday",  2],
            [DateTime.local(2024, 1, 27, opts), "Saturday",  4],
            [DateTime.local(2024, 1, 29, opts), "Monday",    5],
            [DateTime.local(2024, 2, 4, opts), "Sunday",     6],
            [DateTime.local(2024, 2, 29, opts), "Thursday",  9],
            [DateTime.local(2024, 3, 1, opts),  "Friday",    9],
            [DateTime.local(2024, 3, 30, opts), "Saturday", 13],
            [DateTime.local(2024, 3, 31, opts), "Sunday",   14],
            [DateTime.local(2024, 12, 22, opts), "Sunday",  52],
            [DateTime.local(2024, 12, 28, opts), "Saturday",52],
        ];

        test.each(cases)(
            "given %p date, on %p day, returns %p week number",
            (givenDate, day, expectedResult) => {
                const result = getWeekNumber(givenDate as DateTime);
                expect(result).toEqual(expectedResult as number);
            }
        );
    })

Here's some screenshots that I can quickly doublecheck the date with the week number:

Screenshot 2024-01-24 at 21 32 56 Screenshot 2024-01-24 at 21 37 36

Appears only test cases that fall on Sundays are incorrect, specifically getting 1 less that I'm expecting.

Screenshot 2024-01-24 at 22 04 14

To Reproduce Please share a minimal code example that triggers the problem:

const day = DateTime.local(2024, 1, 7, { locale: "en-US" })
console.log(day.weekNumber)

Actual vs Expected behavior A clear and concise description of what you expected to happen.

Given that I've set the first day of the week to Sunday (via firstDay: 7 in Settings), these test cases with days on Sunday, I'm expecting weekNumber to be +1 more than I'm actually seeing.

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

diesieben07 commented 5 months ago

weekNumber and associated properties are always based on the ISO week date. Settings.defaultWeekSettings and the locale only influence localeWeekNumber, etc.

weklund commented 5 months ago

Ah I see, so using localeWeekNumber in my case is ok because this always runs on the user's local device. All tests pass now.

Thank you!