-
Notifications
You must be signed in to change notification settings - Fork 768
Description
Describe the bug
DateTime, Duration (and possibly others) do not accept weekSettings in .reconfigure method, even though the cloning of the locale inside can handle it.
@types/luxon has a wrong definition or a mismatch with the description of the method.
To Reproduce
Settings.defaultWeekSettings = { firstDay: 4, minimalDays: 1, weekend: [], };
let firstDate = DateTime.now();
console.log(firstDate.startOf('week', { useLocaleWeeks: true }));
let b = firstDate.reconfigure({ weekSettings: { firstDay: 6, minimalDays: 1, weekend: [1, 2]} });
console.log(b.startOf('week', { useLocaleWeeks: true }));
Actual vs Expected behavior
Actual: Both dates return the same start day.
Expected: Either typings should be updated to reflect that weedSettings are not supported or (my preferred solution ) reconfigure method should accept weekSettings and pass them to Location.clone().
Desktop (please complete the following information):
- OS: Windows 11
- Browser: Chrme Version 142.0.7444.163 (Official Build) (64-bit)
- Luxon version: 3.7.2
- Your timezone: Europe/Sofia
Additional context
So we have this product where people are working in all parts of the world and need to collab on the same things. Given that we have a lot of personal settings related to Timezone/Start of week/Locale that we override the user`s machine settings with. I am currently doing a migration to Luxon from MomentJS (apparently Luxon deals best with timezones and locales from all other non-moment libraries).
So long story short I have a case where I need to recalculate some date ranges and some days when a user changes their start of week setting and noticed the above behavior.
I looked at the code and noticed that if the parameter gets added to reconfigure it should work fine downstream when cloning the locale.
Im willing to do a PR to add weekSettings as a parameter, but I wasn't sure if it would be accepted.