Implement getDefaultOptions, setDefaultOptions #3069
Conversation
leshakoss
force-pushed
the
v2-setDefaultOptions
branch
from
f1fa984
to
ebbbe69
Compare
leshakoss
force-pushed
the
v2-setDefaultOptions
branch
from
11e86ff
to
343b2f9
Compare
src/endOfWeek/index.ts
Outdated
| const weekStartsOn = toInteger( | ||
| options?.weekStartsOn ?? | ||
| options?.locale?.options?.weekStartsOn ?? | ||
| _defaultOptions.weekStartsOn ?? | ||
| _defaultOptions.locale?.options?.weekStartsOn ?? | ||
| 0 | ||
| ) |
There was a problem hiding this comment.
I'm comparing this PR to #3087 and this one, with the wider scope, feels more future-prone.
If we decide to move locales to separate packages and make Intl the default approach, the change will still make sense. Furthermore, we might think of option autodetection and apply user-specific defaults for weekStartsOn, which would be impossible if we stick to locale-only.
It is indeed more code, but we can approach it slightly differently and reduce it to something like:
| const weekStartsOn = toInteger( | |
| options?.weekStartsOn ?? | |
| options?.locale?.options?.weekStartsOn ?? | |
| _defaultOptions.weekStartsOn ?? | |
| _defaultOptions.locale?.options?.weekStartsOn ?? | |
| 0 | |
| ) | |
| const weekStartsOn = getOption(options, 'weekStartsOn') |
That will also help to abstract the default behavior that we repeat from function to function.
src/_lib/defaultOptions/index.ts
Outdated
| export type AllOptions = LocaleOptions & | ||
| WeekStartOptions & | ||
| FirstWeekContainsDateOptions & | ||
| StepOptions & | ||
| RoundingOptions & | ||
| RepresentationOptions & | ||
| Omit<FormatDurationOptions, 'format'> & { | ||
| format?: FormatOptions['format'] | FormatDurationOptions['format'] | ||
| } & FormatRFC3339Options & | ||
| AdditionalDigitsOptions & | ||
| AdditionalTokensOptions & | ||
| FormatDistanceOptions & | ||
| FormatDistanceStrictOptions & | ||
| RoundToNearestMinutesOptions & | ||
| AreIntervalsOverlappingOptions |
There was a problem hiding this comment.
Maybe we don't need to make all options global, but locale, weekStartsOn, etc.
There was a problem hiding this comment.
Reduced the scope to the big three: locale, weekStartsOn, firstWeekContainsDate
…stWeekContainsDate
| const locale = options?.locale ?? defaultOptions.locale ?? defaultLocale | ||
| const format = options?.format ?? defaultFormat | ||
| const zero = options?.zero ?? false | ||
| const delimiter = options?.delimiter ?? ' ' |
There was a problem hiding this comment.
I'm afraid that this can be a breaking change, but I don't mind it haha
|
When might we expect a new version to be published with those changes, if I dare to ask? :-) |
|
This closed #816 |
|
@olee published in v2.29: https://github.com/date-fns/date-fns/releases/tag/v2.29.1 |
No description provided.