fix(isoWeek): correct week number when the year starts at a UTC offset change - #3201
fix(isoWeek): correct week number when the year starts at a UTC offset change#3201dylanpulver wants to merge 1 commit into
Conversation
…t change
isoWeek() truncated the week difference between the current week's Thursday
and the year's first Thursday. getYearFirstThursday() derives that date from
startOf('year'), which is not local midnight in a zone whose UTC offset
changed at 00:00 on 1 January: Nepal moved +05:30 -> +05:45 at 1986-01-01
00:00, so startOf('year') resolves to 00:15 and every subsequent Thursday
carries that 15 minutes. The truncating diff then loses a whole week.
Under TZ=Asia/Kathmandu, 357 of the 365 days of 1986 returned an isoWeek one
lower than ISO 8601 gives, e.g. dayjs('1986-06-15').isoWeek() was 23 (24 is
correct - 1986-01-01 was a Wednesday, so week 01 is Dec 30 1985 - Jan 5 1986
and Jun 15 is the last day of week 24).
Both operands are Thursdays, so the true difference is a whole number of
weeks; rounding the float diff recovers it regardless of the time-of-day
skew, while diff() already compensates the UTC offset delta.
Adds a `test-isoweek-tz` script running the isoWeek suite under
TZ=Asia/Kathmandu, since the assertions cannot fail in a UTC runner.
nrps9909
left a comment
There was a problem hiding this comment.
Reviewed e6f20e244622e8999eaeea938480d39e9ffe853a.
Independently reproduced the Kathmandu failure on base 73017be: 357 incorrect week numbers in a 1980–2040 sweep. On this head I checked every calendar day at local 00:00, 12:00 and 23:00, both as local and UTC instances, under Asia/Kathmandu, UTC, Asia/Gaza and America/New_York: 534,744 comparisons, zero mismatches.
The reference calculation used native UTC calendar arithmetic: move the instance's actual year/month/date to its ISO Thursday, then count weeks from January 1 of that Thursday's year. In particular, it compared against the date actually represented by a local instance, rather than the nominal input date across skipped or ambiguous midnights. That also removes the Gaza false positives described in the PR.
The Kathmandu isoWeek suite, all 93 suites / 795 tests, the eight existing timezone test legs, and lint passed locally (Node 26.8.1, macOS). I found no blocking regression in the rounding change. This is local verification; no upstream check results were exposed for this head at review time.
isoWeek()returns a week number one too low for a whole calendar year in any timezone whoseUTC offset changed at 00:00 on 1 January.
1986-01-01 was a Wednesday, so ISO 8601 week 01 is 1985-12-30..1986-01-05 and 1986-06-15 is the
last day of week 24 (ISO 8601 §3.1.1.9: week 01 is the week containing the year's first Thursday).
getYearFirstThursday()builds the reference date fromstartOf('year')(
src/plugin/isoWeek/index.js:6-13). In Kathmandu that resolves to1986-01-01T00:15+05:45, notmidnight, and the +4-day Thursday inherits the 15 minutes.
isoWeek()then didnowWeekThursday.diff(diffWeekThursday, W) + 1(:30), anddifftruncates — so23 weeks minus 15 minutesbecame 22.Both operands are Thursdays, so the true difference is a whole number of weeks;
diffalreadycompensates the UTC offset delta (
index.js:353,369), leaving only a sub-day skew. Rounding thefloat diff recovers the integer.
Found by a sweep, not a bug report: every date from 1970-2050 (29,585) checked against an ISO
8601 week-date reference written from the standard, under 18 timezones.
Measured (same commands, same checkout):
npm testThe Gaza rows are not an isoWeek defect and are present before and after:
dayjs('2040-10-22')sometimes resolves to
2040-10-21T23:00+02:00because local midnight that day is ambiguous. Forthe instant it actually holds, week 42 is right; my reference used the nominal date.
A mutant you should know about.
startOf(D)on both Thursdays instead — the first fix Itried — also passes the whole suite including the test added here, but takes Gaza from 2 to 7,
because Gaza's DST starts at 00:00 so
startOf('day')is not midnight there either. So the newtest does not cover that dimension; the sweep is what caught it. Reverting only
:30and keepingthe test fails the Kathmandu leg and restores all 357.
The assertions cannot fail in a UTC runner, so
test-isoweek-tzruns the isoWeek suite underTZ=Asia/Kathmandu, alongside the existing per-timezone legs.
Not covered:
week()(weekOfYear) has a related but different problem I did not touch — underTZ=Pacific/Kiritimati,
dayjs('1994-12-26').week()returns 1 where moment returns 53, because1994-12-31 did not exist there. Happy to open that separately.
Disclosure: the fix, the test and this description were drafted with Claude Opus 5.