Settings.defaultWeekSettings not working

[aronsuarez]

Describe the bug
Settings.defaultWeekSettings has no effect at all, the weeks are starting still at Sunday

To Reproduce
Settings.defaultLocale = 'de-CH'
Settings.defaultWeekSettings = { firstDay: 1, minimalDays: 1, weekend: [6, 7] }

Actual vs Expected behavior
I want to set default start day of the week to monday, currently is nothing changed when i set the Settings.defaultWeekSettings

Desktop (please complete the following information):

• OS: Android, iOS, MacOS (all latest)
• Browser Chrome and Safari Latest
• Luxon version 3.4.4
• Your timezone Europe/Zurich

Additional context
Add any other context about the problem here.

[diesieben07]

In my tests this works correctly. Are you setting useLocaleWeeks when using startOf? startOf('week') on its own always uses the ISO week year.

[gkubisa]

In my opinion having to opt-in to use the default week settings (by passing useLocaleWeeks: true into startOf/endOf) causes friction, is not intuitive and is not consistent with how the other Settings work.

Would it be possible to have the useLocaleWeeks option default to true? Or even better - replace the useLocaleWeeks with a firstDayOfWeek option in startOf/endOf?

[diesieben07]

Would it be possible to have the useLocaleWeeks option default to true?

No, because that would be a breaking change.

Or even better - replace the useLocaleWeeks with a firstDayOfWeek option in startOf/endOf?

Sure, I'd be fine with accepting a PR that adds this as an additional option. However this option not just applies to startOf and endOf. There are other functions which call startOf and endOf (such as hasSame or Interval#count) which will need their docs updated, too.

[gkubisa]

No, because that would be a breaking change.

Well, I assumed it would be the case.

And how about adding useLocaleWeeks to WeekSettings?

With that option we could write this:

Settings.defaultWeekSettings = {
  useLocaleWeeks: true,
  firstDay: 7,
  minimalDays: 4,
  weekend: [6, 7],
};

DateTime.local(2024, 9, 10).startOf('week');

which would be equivalent to this:

Settings.defaultWeekSettings = {
  firstDay: 7,
  minimalDays: 4,
  weekend: [6, 7],
};

DateTime.local(2024, 9, 10).startOf('week', { useLocaleWeeks: true });

[diesieben07]

I personally don't like this approach and would prefer to have as little "behavior altering Settings" as possible.
Settings.throwOnInvalid has been a thorn in my eye ever since I started using Luxon, but it cannot be removed without a thorough change of how invalidity is handled, which will require a new major version. I would prefer it, if something like it weren't added.
The same goes for Settings.twoDigitCutoffYear.

[gkubisa]

I fully support the point about avoiding settings which change the API contract, for example Settings.throwOnInvalid and Settings.twoDigitCutoffYear.

At the same time I find the settings which define the locale-dependent preferences extremely useful, for example Settings.defaultZone, Settings.defaultLocale, Settings.defaultNumberingSystem and Settings.defaultOutputCalendar.

In my opinion Settings.defaultWeekSettings is another example of a very useful locale-dependent preference.

The problem I see with Settings.defaultWeekSettings is that it is not used by default like the other Settings.default* preferences. The fact that I need to opt-in to use the default week settings every time I call startOf/endOf (by passing in { useLocaleWeeks: true }) removes a major benefit of Settings.defaultWeekSettings.

The minimum I'm trying to achieve is to be able to call dateTime.startOf('week')/dateTime.endOf('week') without any extra params and with the first day of the week defined by Settings.defaultWeekSettings.firstDay. Ultimately I'd like Settings.defaultWeekSettings to be handled consistently with the other Settings.default* settings, in the sense that Settings.defaultWeekSettings are used by default with the ability to override them on a case by case basis.

Here's how I think we could approach it.

Step 1

Add useLocaleWeeks to WeekSettings, so that we could avoid having to pass { useLocaleWeeks: true } into startOf/endOf every time. While not perfect, this step solves the main issue (Settings.defaultWeekSettings not used by default) in a very simple and backward compatible way.

Step 2

• Add a firstDayOfWeek: number option to DateTime#startOf, DateTime#endOf and Interval#count. This will allow users to override the week settings on a case by case basis, which would also address Add optional startOfWeek param for startOf() and endOf() methods #1573.
• Deprecate useLocaleWeeks in Settings.defaultWeekSettings, DateTime#startOf, DateTime#endOf and Interval#count.

Step 3

• Remove useLocaleWeeks from Settings.defaultWeekSettings, DateTime#startOf, DateTime#endOf and Interval#count.
• Release a new major version of Luxon.

End result

• WeekSettings will be as they are today.
• useLocaleWeeks: boolean will be completely gone.
• DateTime#startOf, DateTime#endOf and Interval#count will support firstDayOfWeek: number.
• ThefirstDayOfWeek: number option in DateTime#startOf, DateTime#endOf and Interval#count will default to Settings.defaultWeekSettings.firstDay, with the fallback to 1, if the setting is not provided.

What are your thoughts about the above? How would you approach avoiding the need for { useLocaleWeeks: true } in startOf/endOf calls?

[diesieben07]

useLocaleWeeks is not a locale-dependent setting. It changes how startOf/endOf works. If enabled, it uses weeks in terms of the locale, if disabled it uses the ISO week year system.
It will never be completely removed, because that would be removing a feature (using ISO weeks).

How would you approach avoiding the need for { useLocaleWeeks: true } in startOf/endOf calls?

I wouldn't. You don't always want locale-based weeks.

[gkubisa]

Could Luxon allow developers to set a preference globally to determine, if they want locale-based or ISO-based weeks by default, without having to set it every time when calling DateTime#startOf, DateTime#endOf and Interval#count?

[diesieben07]

That is exactly what I would like to avoid as explained in my previous comment.

To me this is the same as globally configuring that startOf suddenly by default always uses startOf('day'). Global configuration like this makes it hard to reason about what your application is doing and makes it hard to write code that depends on Luxon and works in the presence of other code that also uses Luxon.

If I am a library author and my library is supposed to work in conjunction with Luxon, I would no longer be able to rely on Luxon's API, because users can just configure it to operate differently.

[gkubisa]

I see now that adding Settings.defaultWeekSettings.useLocaleWeeks would be a breaking change. I'm not going to push it further but please consider the following when planning the next major release of Luxon:

Settings.defaultWeekSettings should be consistent with the other Settings.default* preferences, in particular:

• Settings.defaultWeekSettings should apply by default (without having to opt-in using useLocaleWeeks: true).
• Settings.defaultWeekSettings should be possible to override on a case-by-case basis using function params, for example useLocaleWeeks (or firstDayOfWeek).

Thank you for the great insights and quick replies.

[diesieben07]

Settings.defaultWeekSettings should apply by default (without having to opt-in using useLocaleWeeks: true).

That is not what useLocaleWeeks does! useLocaleWeeks does not say "Use Settings.defaultWeekSettings". useLocaleWeeks says "Use weeks based on the locale, not standard ISO weeks". By default, this looks at the device's locale. defaultWeekSettings overrides the device's locale, similar to how defaultZone overrides what the default time zone of the device is. Configuring defaultZone has no effect on things that do not use the local time zone and in exactly the same way defaultWeekSettings has no effect on things that do not use the local week configuration.

Settings.defaultWeekSettings should be possible to override on a case-by-case basis using function params, for example useLocaleWeeks (or firstDayOfWeek).

A pull request for this would have my support for sure.