Skip to content

[scheduler] Type the plain dates through the BaseTemporal namespace - #24034

Merged
flaviendelangle merged 11 commits into
mui:masterfrom
flaviendelangle:scheduler-base-temporal-namespace
Oct 30, 2026
Merged

flaviendelangle merged 11 commits into
mui:masterfrom
flaviendelangle:scheduler-base-temporal-namespace

Conversation

@flaviendelangle

@flaviendelangle flaviendelangle commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

The Scheduler now reads its plain dates the way the Base UI version of BaseTemporal exposes them. Nothing is exported publicly yet.

Changes

  • The plain dates are typed as BaseTemporal.PlainDate. It's the type the namespace exposes, instead of the PlainDate class. The internal exports of @mui/x-scheduler-internals/internals give BaseTemporal instead of the PlainDate and ZonedDateTime classes.
  • The statics are called through the namespace, for example BaseTemporal.PlainDate.from(), BaseTemporal.PlainDate.compare() and BaseTemporal.ZonedDateTime.from().
  • The day numbers are read through getDayNumber(), because the interface has no dayNumber. The internal exports give it next to BaseTemporal.
  • The namespace comes with the copy. base-temporal/index.ts and base-temporal/index.parts.ts are copied as is from the Base UI version. No file that was already in the copy changes.
  • The Scheduler only uses the public API of BaseTemporal, so it can import BaseTemporal from the package of the library:
    • plain-date-utils/dayNumber.ts owns the ISO arithmetic of the day numbers: isoDateToDayNumber() and dayNumberToIsoDate(), the start of a week or a month, the addition of months, MS_PER_DAY and LAST_SUPPORTED_DAY. They replace the internals of the copy.
    • getDayNumber() reads the ISO fields of a plain date, and plainDateFromDayNumber() creates one with the BaseTemporal.PlainDate constructor. Both cache their results: the plain dates are immutable, and the views read the same days on every render. A cached read is a WeakMap or Map lookup instead of a field read. The first read of a date that add() or from() creates, and a date missing from the cache, cost more than the internal field and creation. The bundled benches run with warm caches.
    • getDayListFromPlainDates() walks the day numbers, so its days come from the cache.
    • PlainDateDuration is the parameter of BaseTemporal.PlainDate#add().
    • ESLint zones (import/no-restricted-paths) forbid the other modules of the copy in the packages and in test/. They join the zones of all the files, and the copy keeps only the zones of the internals index.
    • toEqualPlainDate() checks the public fields of a plain date instead of its internal day number.
  • Simplifications on the lines this touches.
    • The loops read each day number once.
    • The code that built a plain date only to read its day number back uses the day-number helpers: toLocalDayNumber(), getStartOfWeekDayNumber(), getStartOfMonthDayNumber(), getStartOfYearDayNumber(), addMonthsToDayNumber() and getDayCount(). The private getStartOfRRuleWeek() goes.
    • Outside the loops of the occurrences, the plain dates are compared with BaseTemporal.PlainDate.compare() and equals(), and moved by a day with add().
    • getDaysTheOccurrenceIsVisibleOn() takes the day numbers of the visible days, read once by its caller for all the occurrences. Reading them through getDayNumber() for each occurrence and each day made the month view case of the grouped-by-day bench 6% slower than the field read.

No public change: BaseTemporal and getDayNumber() are only exported from @mui/x-scheduler-internals/internals, which also stops exporting the PlainDate and ZonedDateTime classes. It's a published entry point, but only for the Scheduler packages. The models that hold plain dates, such as SchedulerProcessedDay, EventCalendarFetchRange and the preset definitions of the Timeline, aren't re-exported by @mui/x-scheduler or @mui/x-scheduler-premium.

Bundle size

Measured from source with Rolldown, minified, with third-party packages external and without the error minifier, gzip level 9.

Entry master This PR Difference
event-calendar 70.18 kB 70.23 kB +57 B
event-calendar-premium 84.66 kB 84.32 kB −341 B
event-timeline-premium 61.43 kB 61.54 kB +110 B

esbuild keeps every class of a namespace when the namespace is re-exported by name. The entries that don't use them grow there: +3.14 kB gzipped for event-calendar and +3.13 kB for event-timeline-premium, which now include PlainTime, PlainDateTime and ZonedDateTime. Rolldown (the bundler of Vite) and webpack 5 drop them. A reduced case of the same re-exports confirmed this.

Validation

  • pnpm test:unit --project "x-scheduler*" --run and pnpm test:browser --project "x-scheduler*" --run pass.

  • The benches were run bundled with Rolldown, as the Vitest module runner reads every imported function through a getter: under Vitest, the getDayNumber() calls of the loops cost up to 30% that a bundle doesn't pay. Bundled, they stay within their noise, apart from the header walk of the Timeline, which builds fewer plain dates: iterate() is about twice as fast for dayAndMonth and monthAndYear. Reading the plain dates through their public API doesn't move the benches either, as the day numbers and the dates come from the caches.

  • A /simplify pass on the change made the simplifications above.

  • A review round with Codex and the base-ui-review skill, on the first commits, approved with one finding rejected. add({ days: -1 }) throws on the first supported date, the edge of the supported range.

  • A second round, on the move to the public API, found that countBefore() built the day before the start of a daily series with an interval of millennia, before the supported days. createPlainDate() built it without checking it, but the BaseTemporal.PlainDate constructor this PR uses rejects it. It's fixed and tested.

  • A third round found that the Scheduler's ESLint zone replaced the one of all the files, which forbids importing the internals/index.ts barrel. The zones of the copy are now part of the rule of all the files.

  • A review found that the validating constructor made the instants at the end of 9999 throw east of UTC, such as a "no end date" UNTIL=99991231T235959Z, exDate or event end in Tokyo, which fall on January 1st, 10000. [scheduler] Sync the BaseTemporal copy with the Base UI version #24040 brings the days of Date to the copy, so these instants give dates of the year 10000, and LAST_SUPPORTED_DAY moves to September 13th, 275760. The last 31-day period of skipped days stops at that day, as the day after it has no plain date. Tests cover the UNTIL, the exDate and the event end in Tokyo.

  • A second review found that the monthly and yearly days after September 13th, 275760 threw. They return no occurrence now, like the weekly ones. Unlike Temporal, the first instant of Date throws in the zones west of UTC, where it falls on the day before the first supported day.

  • New tests: the ISO arithmetic of the day numbers on every day of the years 0 to 9999 and every 9973rd day of Date, the caches of the plain dates and of their day numbers, the dates created by the methods of the plain dates and the dates in another calendar.

  • TypeScript, ESLint and Prettier pass.

  • I have followed (at least) the PR section of the contributing guide.

🤖 Generated with Claude Code

@flaviendelangle flaviendelangle added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: scheduler Changes related to the scheduler. labels Oct 28, 2026
@code-infra-dashboard

code-infra-dashboard Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Deploy preview

https://deploy-preview-24034--material-ui-x.netlify.app/
QR code for https://deploy-preview-24034--material-ui-x.netlify.app/

Bundle size

Bundle Parsed size Gzip size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium 0B(0.00%) 0B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)
@mui/x-scheduler 🔺+268B(+0.06%) 🔺+44B(+0.04%)
@mui/x-scheduler-premium 🔺+465B(+0.08%) ▼-123B(-0.07%)
@mui/x-chat 0B(0.00%) 0B(0.00%)
@mui/x-license 0B(0.00%) 0B(0.00%)

Details of bundle changes


Check out the code infra dashboard for more information about this PR.

@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@flaviendelangle
flaviendelangle force-pushed the scheduler-base-temporal-namespace branch from c1210ae to 6eb4fbe Compare October 28, 2026 14:17
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 28, 2026
@flaviendelangle
flaviendelangle marked this pull request as ready for review October 29, 2026 09:36
@rita-codes

Copy link
Copy Markdown
Member

PR review

One merge-blocking regression, and it comes from the one semantic change hidden in this otherwise mechanical migration: plainDateFromDayNumber() used to be the unvalidated createPlainDate() and now goes through the validating BaseTemporal.PlainDate constructor. Every instant the Scheduler ingests passes through it, so an event, exDate or UNTIL on 9999-12-31 late UTC in a timezone east of UTC now throws inside processEvent() / new RecurringSeries(), where it worked before. The same change explains the edge throws the description lists as accepted, and the countBefore() "fix", which guards a hazard this PR introduced. Everything else is in good shape: tsc, ESLint and Prettier are clean, the 4045 scheduler unit tests pass under TZ=UTC, the two new copy files are byte-identical to mui/base-ui-plus feat/plain-day at 578caaf86, the ESLint zone fires for every forbidden import I tried (including from test files), and no .dayNumber read or non-index import of the copy survives outside it.

Bugs (3)

1. 🔴 Instants past 9999-12-31 in the event's timezone now throw while processing events

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/plainDate.ts:27-28, reached from plain-date-utils/timeZone.ts:124-126 (toPlainDate) and plain-date-utils/adapter.ts:151

const isoDate = dayNumberToIsoDate(dayNumber);
plainDate = new BaseTemporal.PlainDate(isoDate.year, isoDate.month, isoDate.day);

Before the PR this was createPlainDate(dayNumber, ISO_CALENDAR_ID), which builds a date outside the supported years without throwing (it still does on master: createPlainDate(LAST_SUPPORTED_DAY + 1) returns +010000-01-01). toPlainDate(epochMs, timezone) is the only bridge from user instants to plain dates, and it has no range check, so the validating constructor is now the first thing that sees a day in year 10000. Reproduced with the date-fns adapter (probe script in my notes):

  • parseRRule(adapter, 'FREQ=DAILY;UNTIL=99991231T235959Z', 'Asia/Tokyo') then new RecurringSeries(...) throws "PlainDate received the year 10000" at RecurringSeries.ts:333 (this.untilDay = toDay(rule.until)). Same rule in UTC or America/New_York works, and UNTIL=99991231T000000Z in Tokyo works, so it is the east-of-UTC day rollover.
  • processEvent({ start: '9999-12-31T23:00:00Z', end: '9999-12-31T23:30:00Z' }, 'Asia/Tokyo', adapter) throws the same way via buildProcessedDate → plainDateFromDayNumber.
  • An exDate at 9999-12-31T23:00Z in Tokyo throws at RecurringSeries.ts:334.

On the merge base all three produce an unvalidated 10000-01-01 and the views work, because every consumer compares day numbers. RecurringSeries.fromEvent is the first line of getRecurringEventOccurrencesForVisibleDays and nothing in the selectors catches, so the whole calendar throws for that one event.

Failure scenario: a backend that uses 9999-12-31 as its "no end date" sentinel (SQL Server datetime max, SAP, several iCal exporters emit UNTIL=99991231T235959Z) feeds one recurring event to an Event Calendar whose events are in Tokyo or Sydney. The calendar crashes on first render; on master it rendered.

Fix: keep plainDateFromDayNumber() strict (the new test at plainDate.test.ts:21-23 shows that's deliberate) but clamp at the bridge: in toPlainDate(), plainDateFromDayNumber(Math.max(FIRST_SUPPORTED_DAY, Math.min(toLocalDayNumber(epochMs, timezone), LAST_SUPPORTED_DAY))), with FIRST_SUPPORTED_DAY = isoDateToDayNumber(0, 1, 1) next to LAST_SUPPORTED_DAY in dayNumber.ts. A day clamped to 9999-12-31 compares the same as 10000-01-01 did for every view that can exist. Alternatively keep untilDay and excludedDays as day numbers, which the recurrence engine already prefers (excludedDays is a Set<number>).

2. 🟡 The recurrence engine throws in the first and last 18 days of the supported range, where it didn't before

Location: packages/x-scheduler-internals-premium/src/internals/utils/recurring-events/RecurringSeries.ts:503-506 (isSkippedDay), :190-195 (getFirstFrom, WEEKLY), :386 (hasOccurrenceBefore)

skippedDays = getSkippedDays(
  plainDateFromDayNumber(period * SKIPPED_DAYS_PERIOD),
  plainDateFromDayNumber((period + 1) * SKIPPED_DAYS_PERIOD),

The description lists these as "the edge of the supported range, like the dates after 9999", but they are regressions of this PR, not pre-existing limits: on the merge base all of them go through the unvalidated createPlainDate and return normally. Reproduced:

  • YEARLY series from 9998-12-25, getOccurrenceDays(9999-01-01, 9999-12-31) throws "year 10000, month 1, day 14" from isSkippedDay (the last 31-day period starts 9999-12-14 and its exclusive end is 14 days past LAST_SUPPORTED_DAY). Any occurrence on 9999-12-14..31 trips it; one on 9999-12-13 doesn't.
  • WEEKLY BYDAY=SA from 0000-01-01 throws "year -1" at :191-195: the Monday-based week start lands before year 0.
  • DAILY from 0000-01-05 throws from the period-0 start of isSkippedDay; hasOccurrenceBefore(0000-01-01) throws at :386 (day.add({ days: -1 }), was plainDateFromDayNumber(day.dayNumber - 1)).

Month and week views ending on 9999-12-31 already threw on master (getPlainDateWeekStart(...).add({ days: 6 }) validates in both versions), so the new crash window is a day view or an agenda range between 9999-12-14 and 9999-12-30 with a recurring event, or Jan 1–18 of year 0. Not a realistic UI path, but the engine is a library and addDays/addMonths already guard their own range; the three sites above are the ones left without a guard.

Failure scenario: getOccurrenceDays() of a yearly series in December 9999 throws instead of returning the occurrence.

Fix: compute the period bounds on day numbers and clamp them (Math.max(period * 31, FIRST_SUPPORTED_DAY), Math.min((period + 1) * 31, LAST_SUPPORTED_DAY + 1)); getSkippedDays() only calls getDayNumber() on its arguments, so it can take the numbers directly. For WEEKLY and hasOccurrenceBefore, reuse the local addDays() pattern (return null outside the range) instead of .add(). Then the "first/last 18 days" caveat can go from the description.

3. ℹ️ MiniCalendar compares the active day with equals() and today with getDayNumber()

Location: packages/x-scheduler/src/event-calendar/mini-calendar/MiniCalendar.tsx:280-281

const isToday = getDayNumber(day.plainDate) === nowDayNumber;
const isActive = day.plainDate.equals(visiblePlainDate);

equals() also compares calendarId. Both operands are ISO today (visiblePlainDate comes from toPlainDate, the days from plainDateFromDayNumber), so no behavior change, but the pair reads as if the two checks had different semantics, and the day a non-ISO visiblePlainDate appears isActive goes silently false. getDayNumber(day.plainDate) === getDayNumber(visiblePlainDate) matches the line above.

Tests (3)

1. 🟡 No test pins the range edges the PR changes

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/plainDate.test.ts:21-23, packages/x-scheduler-internals-premium/src/internals/utils/recurring-events/RecurringSeries.test.ts:107-152

expect(() => plainDateFromDayNumber(3_000_000)).to.throw();

The only range test uses a value 67 000 days past the limit. Nothing asserts that plainDateFromDayNumber(LAST_SUPPORTED_DAY) succeeds and LAST_SUPPORTED_DAY + 1 throws, nor that a day before 0000-01-01 throws. The describe('last supported day') block stops before 9999-12-14, so it never reaches the isSkippedDay period that throws (bug 2), and no test covers an UNTIL or event end on 9999-12-31 in an east-of-UTC timezone (bug 1). Whatever the fix for those two, a test for each is the thing that keeps the next refactor of plainDateFromDayNumber from reopening them.

Failure scenario: UNTIL=99991231T235959Z with timezone: 'Asia/Tokyo' through parseRRule + new RecurringSeries(), and a YEARLY series expanded over December 9999.

2. 🟡 Padding and week-start coverage is partial

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/timeZone.test.ts:86-99, plain-date-utils/dayNumber.test.ts:45-62

toWallClockIsoString now hand-rolls the time with padStart; the tests assert 02:30:15 and 00:15:10, so a formatter that padded only the hour would pass. getStartOfWeekDayNumber is tested for firstDayOfWeek 1, 3, 6 and 7 only, and AgendaView.tsx:242 now calls it directly, bypassing the checkWeekStartsOn dev check. One wall-clock 09:05:03 case and the values 2, 4 and 5 close both gaps.

3. ℹ️ The description credits tests that aren't in the diff, and one that guards a self-inflicted hazard

timeZone.test.ts and date-utils.test.ts only have the PlainDate → BaseTemporal.PlainDate rename and a Prettier re-wrap, no new assertions. The "millennia" test at RecurringSeries.test.ts:208 is good, but what it guards is this PR: on the merge base countBefore() built start - 1_000_000 days through the unvalidated createPlainDate and returned normally (the value is unused when count === 0). Worth rewording the validation section so the next reader doesn't look for a pre-existing bug.

Simplifications (5)

1. 🟡 Put dayNumber on SchedulerProcessedDay instead of threading a parallel array

Location: packages/x-scheduler-internals/src/internals/utils/event-utils.ts:110-128, use-event-occurrences-grouped-by-day/useEventOccurrencesGroupedByDay.ts:98-100, get-day-list/getDayList.ts:28-36

const dayNumbers = days.map((day) => getDayNumber(day.plainDate));
for (const occurrence of occurrences) {
  const eventDays = getDaysTheOccurrenceIsVisibleOn(occurrence, days, dayNumbers);

getDayListFromPlainDates() is the only producer of SchedulerProcessedDay (seven callers, all through it, no literal in tests), and its loop already holds dayNumber. Adding it to the day (days.push({ ...createProcessedDate(...), plainDate, dayNumber })) gives the field read the author wanted for the grouped-by-day bench without the third positional parameter, whose "same length and order as days" contract is only enforced by convention (a shorter array makes undefined < x and undefined > x both false, so every remaining day gets pushed). It also replaces the getDayNumber(day.plainDate) lookups at MonthViewCell.tsx:200, MiniCalendar.tsx:280, AgendaView.tsx:243,269, DayTimeGrid.tsx:406-407, useEventOccurrencesWithDayGridPosition.ts:134 and useCalendarGridPlaceholderInDay.ts:67. The type isn't re-exported by @mui/x-scheduler or the premium package, as the description notes. Net about −12 lines and one footgun fewer.

2. 🟡 The cold paths of the two caches are 7–16x slower than the field reads they replace, and the benches only run warm

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/plainDate.ts:21-50

dayNumber = isoDateToDayNumber(isoPlainDate.year, isoPlainDate.month, isoPlainDate.day);

Measured with plain node + tsx (no Vitest runner, median of 5), ns per operation:

Operation master PR
day number of a cached date 4.8 (.dayNumber) 20.3 (WeakMap hit)
day number of a fresh date (add()/with()/from() result) 4.8 254.9
date from a day number, hit 37.9 (createPlainDate) 18.6
date from a day number, miss 37.9 618
compare(a, b) 7.3 32.5

The miss of getDayNumber() reads year, month and day through three getters that each recompute fromDayNumber() on the copy (fields are lazy since #23969), then runs isoDateToDayNumber. Every add()/with()/from() result is a fresh instance, so it always misses: RecurringSeries.ts:377,386,491 (.add({ days: ±1 })), getDayList.ts:24, adapter.ts:62 (createEndOfPlainDate), getRecurringEventOccurrencesForVisibleDays.ts:47, date-utils.ts:84-89 (getWeekNumber, two fresh dates per agenda week row). The miss of plainDateFromDayNumber() pays toInteger ×3, checkIsoDate, checkCalendarRange, Object.freeze, two map writes and the periodic clear(): a WEEKLY series with COUNT started in 1970 and viewed today touches ~2 900 distinct days on first expansion, ~1.8 ms versus ~0.1 ms on master. All of this is sub-frame at the Scheduler's call counts, so no user sees it, but "a cached read takes about as long as the internal field read" is 4x off and the cold cost is exactly what the warm iterate.bench.ts / grouped-by-day benches can't show.

Fix: where the PR itself builds a ±1-day neighbour, use plainDateFromDayNumber(getDayNumber(x) ± 1) so the result is pre-registered in both caches (the five sites above). In getWeekNumber, isoDateToDayNumber(year, 1, minimalDays) replaces getDayNumber(BaseTemporal.PlainDate.from({ year, month: 1, day: minimalDays })); date-utils.ts already imports from ./plain-date-utils/dayNumber and the ESLint zone only restricts the copy. Optionally add a cold-cache bench (the WEEKLY-since-1970 case) so the numbers in the description cover both paths.

3. ℹ️ draggedDay round-trips number → date → number

Location: packages/x-scheduler-internals/src/calendar-grid/day-event/CalendarGridDayEvent.tsx:74,90,207, calendar-grid/day-cell/useDayCellDropTarget.ts:52

The producer builds plainDateFromDayNumber(n) and the only reader does getDayNumber(data.draggedDay). CalendarGridDayEvent.DragData is internal (not SchedulerOccurrencePlaceholderExternalDragData), so a draggedDayNumber: number drops two lookups and both BaseTemporal imports. Pre-existing shape, but the PR touched both ends.

4. ℹ️ Design question: duplicate the ISO math now, or keep plain-date-utils as the one internals-aware layer until the package lands?

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/dayNumber.ts, eslint.config.mjs:293-325

dayNumber.ts is a transliteration of the copy's iso.ts / calendar.ts (toDayNumber, fromDayNumber, getIsoDayOfWeek, getStartOfWeek, getStartOfMonth, addIsoMonths, LAST_SUPPORTED_DAY): 91 lines plus 86 test lines plus the two caches, all so that the Scheduler is package-ready today. The alternative is two lines in the block's ignores for plain-date-utils/{plainDate,dayNumber}.ts, which is what that folder already was on the merge base (it imported createPlainDate and ISO_CALENDAR_ID), and writing the Hinnant code in the PR that swaps the copy for @base-ui/plus. Same end state, the duplication just moves. I'd want to know how close that swap is before picking; the stricter 0..9999 behaviour of plainDateFromDayNumber (bugs 1 and 2) is the one consequence that isn't a wash either way.

5. ℹ️ The esbuild cost of the namespace is +10.6 kB minified, not +3.1 kB

Location: packages/x-scheduler-internals/src/internals/utils/base-temporal/index.ts:1

Measured on a three-line entry that uses only PlainDate.compare and the constructor, minified: esbuild 0.28 emits 25 034 B through export * as BaseTemporal versus 14 436 B importing the class by name; Rolldown 1.2 and webpack 5 give 14 448 / 14 390 B either way. The +3.1 kB in the description is presumably the gzip figure, worth saying. esbuild also turns the namespace into a getter object, so every BaseTemporal.PlainDate access is a call under it. Only esbuild-based consumers pay (Vite dependency pre-bundling in dev, tsup, Bun); Next.js and Vite production builds don't. Fine to accept, as the description does, as long as the unit is explicit. Also: eslint.config.mjs:282-284 and :296-299 spell the two copy directories twice, and the zones a third time; a shared BASE_TEMPORAL_COPY_DIRS constant like INTERNALS_INDEX_RESTRICTED_ZONES would do.

Docs (4)

1. 🟡 The copy's revision, the "public API only" rule and the mirror process are recorded nowhere in-repo

Location: packages/x-scheduler-internals/src/internals/utils/base-temporal/ (no README or header), eslint.config.mjs:293-295, plain-date-utils/plainDate.ts:35-38

The only in-tree hints are the two ESLint comments. The copy at this PR's base isn't the 578caaf86 revision the new index files come from (it still has PlainDateCache and the "calendar grids" comments that #24040 removes), and nobody can tell that without rebuilding and diffing. A README.md in base-temporal/ stating the upstream path and SHA, "change it upstream first, then copy back", and the rule this PR introduces ("import only BaseTemporal from the index; day-number arithmetic lives in plain-date-utils/dayNumber.ts; never read plainDate.dayNumber, it's an internal field the package won't export") would serve both this PR and #24040. The last point belongs in getDayNumber()'s JSDoc too: the class still declares public readonly dayNumber (PlainDate.ts:355), autocomplete offers it, and ESLint can't catch a property read.

Failure scenario: the next contributor reads plainDate.dayNumber, or edits the copy directly and loses the change at the next sync.

2. 🟡 Description accuracy

  • "A second round found that countBefore() built the day before the start of a daily series with an interval of millennia, before the supported years, which the constructor rejects." The constructor only rejects it because this PR made plainDateFromDayNumber validating; on master that line was harmless. Same for "the occurrences of a recurring series throw in the first 18 days of year 0 and the last 18 days of 9999", presented as a pre-existing edge: they don't throw on master (bug 2).
  • "New tests: ... the dates created by the methods of the plain dates" is in plainDate.test.ts, but timeZone.test.ts and date-utils.test.ts gained no assertions.
  • "No public change": true for @mui/x-scheduler and the premium package, but @mui/x-scheduler-internals/internals is a published entrypoint (package.json exports) and loses PlainDate, ZonedDateTime and the PlainDate class. Fine if the internals packages are treated as unversioned, worth one clause.
  • Bundle table: say minified or gzipped.

3. ℹ️ The second ESLint zone message doesn't say what to import instead

Location: eslint.config.mjs:318

message: 'These are internals of the BaseTemporal copy.',

The first zone's message is actionable ("Import BaseTemporal from the index of the copy, and the day number helpers from plain-date-utils"); this one should end with "Use BaseTemporal.ZonedDateTime from internals/utils/base-temporal instead." The block comment at :293-294 says "the BaseTemporal namespace of its index", while except: ['./index.ts'] also allows the index's type exports (PlainDateFields, ZonedDateTimeOptions...), which is the intent; "the exports of its index" matches.

4. ℹ️ Cache and loop comments

  • plainDate.ts:15: the WeakMap line should say why it has no bound (entries die with their date, so it can hold add()/from() results) and that plainDateLookup.clear() leaves it alone. plainDate.ts:18-20 should state the contract callers now lean on: same instance for a day number until the cache is cleared at 10 000 entries, so compare with equals()/day numbers, never ===.
  • getDayList.ts:27 "without building them, as they're cached" overstates a cold cache; "returns the cached instance of each day instead of building one with add()" is what the rewrite buys.
  • models/event.ts:426-427 says consumers "compare its day number with the day numbers of instants" but no longer points at how: getDayNumber(plainDate) vs toLocalDayNumber().
  • plainDate.test.ts:18 "should return the same plain date for a day number" asserts reference identity; "the same instance" says what it tests.

Verdict

Request changes — the validating plainDateFromDayNumber() turns instants past 9999-12-31 in east-of-UTC timezones (a common "no end date" sentinel) into a render-time throw that master handled.


🤖 Review generated with Claude Code

@flaviendelangle
flaviendelangle force-pushed the scheduler-base-temporal-namespace branch from a85420a to fd85452 Compare October 30, 2026 08:01
@flaviendelangle

Copy link
Copy Markdown
Member Author

Thanks for the review. I checked each item against the code; fbbefbd and fd85452 apply the ones that held up. The branch is now stacked on #24040, which syncs the BaseTemporal copy.

Bugs

  1. 🔴 Instants after 9999 east of UTC: confirmed. UNTIL=99991231T235959Z, an exDate and an event end at 23:30Z on 9999-12-31, all in Asia/Tokyo, threw on this branch, while master returned +010000-01-01. I didn't clamp: the Base UI version of BaseTemporal now supports the days of Date, from -271821-04-20 to +275760-09-13, like Temporal. [scheduler] Sync the BaseTemporal copy with the Base UI version #24040 brings that range to the copy, and this PR moves LAST_SUPPORTED_DAY to September 13th, 275760. These instants now give dates of the year 10000. A clamp in toPlainDate() alone would also have missed buildProcessedDate(), which calls plainDateFromDayNumber() directly. New tests cover the three Tokyo cases, in processEvent.test.ts and RecurringSeries.test.ts.
  2. 🟡 Throws at the edges of the range: the wider range removes the year 0 and 9999 cases. At the new last day, the last 31-day period of isSkippedDay() stops at LAST_SUPPORTED_DAY, since the moved last-supported-day tests ran into it. Two spots still throw, at the very start of Date's range only: the WEEKLY week start and hasOccurrenceBefore() on the first day of Date. You're right that these throws weren't on master, and the description no longer says they were.
  3. ℹ️ MiniCalendar: kept. equals() is the plain-date API the Scheduler is moving to. isToday compares day numbers only because the store keeps "now" as one. The Scheduler shows a single calendar at a time, so both dates are in the same calendar.

Tests

  1. 🟡 Range edges: added. plainDateFromDayNumber(LAST_SUPPORTED_DAY) works and LAST_SUPPORTED_DAY + 1 throws. The Tokyo cases are covered (see bug 1). The last-supported-day tests moved to September 275760.
  2. 🟡 Padding and week starts:
    • Added a 09:05:03 case for the padding.
    • I didn't add week starts 2, 4 and 5. getStartOfWeekDayNumber() has no branch per weekday, and the four values already tested cover the same day, the days before it, and the wrap past Sunday.
    • AgendaView still runs the weekStartsOn check, through getWeekNumber() → getPlainDateWeekStart() on the first day of each week.
  3. ℹ️ Test credits:
    • The description only credits new assertions in plainDate.test.ts and dayNumber.test.ts, not timeZone.test.ts or date-utils.test.ts.
    • The countBefore() bullet now says the hazard comes from the validating constructor this PR uses.

Simplifications

  1. 🟡 dayNumber on SchedulerProcessedDay: not applied. The days are moving to plain dates, and a second field would give two ways to read the same day. The parallel array has a single caller, which builds it on the line before.
  2. 🟡 Cold caches:
    • getWeekNumber() now uses isoDateToDayNumber(year, 1, minimalDays).
    • The add({ days: ±1 }) calls stay, since the Scheduler uses add() for date arithmetic. By your numbers, the cold cost stays below a frame.
    • The description no longer says a cached read costs the same as the field.
    • I didn't add a cold-cache bench.
  3. ℹ️ draggedDay: kept. The internal drag data holds plain dates on purpose, since the internal contracts are moving to plain dates.
  4. ℹ️ Duplicating the ISO math: that's a deliberate choice. The Scheduler only uses the public API of the copy, so replacing the copy with the package will only change imports.
  5. ℹ️ esbuild: the description now says the figures are gzipped. The two directories of the copy are now named once in eslint.config.mjs.

Docs

  1. 🟡 README in the copy: not added. The folder stays an exact copy of the Base UI version, and a file of its own would make it diverge. Instead:
    • the ESLint comment of the copy now says to change the Base UI version first, then copy it again;
    • the JSDoc of getDayNumber() says to use it instead of the internal dayNumber field.
  2. 🟡 Description: fixed the countBefore() bullet and the 18-day bullet. It now also says that @mui/x-scheduler-internals/internals stops exporting the PlainDate and ZonedDateTime classes, and that the esbuild figures are gzipped.
  3. ℹ️ ESLint messages: the second zone now points to BaseTemporal.ZonedDateTime, and the block comment says "the exports of its index".
  4. ℹ️ Comments: applied all four.

🤖 Comment generated with Claude

@rita-codes

Copy link
Copy Markdown
Member

PR review (round 2)

Nothing merge-blocking anymore. Round 1's blocker is fixed the right way: with #24040 underneath, UNTIL=99991231T235959Z, an exDate and an event end at 23:30Z on 9999-12-31 all give +010000-01-01 in Tokyo through the date-fns adapter, buildProcessedDate() included, and the new tests pin the processed key. The copy in this branch is byte-identical to its source (the two index files too), TypeScript, ESLint and Prettier pass, and the Scheduler suite passes (213 files, 4,054 tests, TZ=UTC). I also applied the source's newer version of the copy on top of this branch: types and tests still pass, so the pending re-sync is a drop-in.

What's new in this round: the shared 10,000-entry date cache turns long COUNT series into a steady-state cliff that master didn't have, the moved range end creates throws master didn't have either, and the Tokyo regression test doesn't pin what it's named after.

Bugs (7)

1. 🟡 Two long COUNT series thrash the plain-date cache on every recompute

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/plainDate.ts:27 and packages/x-scheduler-internals-premium/src/internals/utils/recurring-events/RecurringSeries.ts:466

    if (plainDateLookup.size >= MAX_CACHED_PLAIN_DATES) {
      plainDateLookup.clear();

countBefore() walks every occurrence from DTSTART to the window (day = ruleDays.getNext(day) → addDays() → plainDateFromDayNumber()), so a COUNT series inserts one cached date per occurrence since its start, on every expansion. One long series fits: WEEKLY MO/WE/FR since 1970 with a COUNT is 8,796 distinct days, DAILY since 2010 in Pacific/Apia with a COUNT 5,914. Two of them exceed 10,000 together, so each recompute clears the Map, evicts the other series' days and the view's ~42, and the next recompute re-walks cold at ~360 ns per date instead of ~6 ns. Measured with the bundled benches in perf-24034-v2/f-thrash (median of 15, master emulation = unvalidated createPlainDate + field reads):

COUNT series since 1970 per render master this PR cache cleared
1 0.51 ms 0.52 ms 0 / 15
2 0.82 ms 6.75 ms 15 / 15
3 1.06 ms 6.10 ms 15 / 15
4 1.24 ms 8.19 ms 15 / 15

Master had no shared state, so no cliff. This is not round 1's cold-fill cost (a one-off, which the reply rightly called sub-frame): here the cache never warms. DAILY without timezone skips takes the arithmetic branch (:479-485) and is unaffected, so the #24018 dataset (COUNT 12 and 400) doesn't hit it. Realistically, >5,000 occurrences needs a weekday series started before 2007 or a thrice-weekly one since 1970; Google and Outlook cap COUNT far below that, so this is a synthetic dataset today, and 8 ms is still under a frame. It is still a design hazard worth closing while the code is fresh: an unbounded insert path behind a bounded global cache.

Failure scenario: a calendar with two or more imported weekly COUNT series created decades ago; every navigation, edit or resource toggle re-expands them at ~3 ms each, 6 to 8x master, forever.

Fix: walk day numbers in countDays/visitOccurrences and build a plain date only for the days handed to visit(). Everything in the loop is already number-pure: isSkippedDay() starts with getDayNumber(day) and ends with skippedDays.has(dayNumber), excludedDays is a Set<number>, the UNTIL compare is a day-number compare, WEEKLY needs dayOfWeek (((n + 3) % 7 + 7) % 7 + 1), MONTHLY needs dayNumberToIsoDate(). That also removes the +24% warm overhead of the WEEKLY walks (two WeakMap reads per step). If the cache must stay as is, a two-generation cache (keep the emptied Map as a read-only fallback) avoids evicting one render's working set.

2. 🟡 MONTHLY and YEARLY series landing after September 13th, 275760 throw

Location: RecurringSeries.ts:261 and :79

        candidate = plainDateFromDayNumber(getDayNumber(month) + this.monthlyTargetDay - 1);
    ? plainDateFromDayNumber(getDayNumber(monthStart) + day - 1)

With the range ending on December 31st, these two could never leave the supported days: monthlyTargetDay <= month.daysInMonth and day <= daysInMonth bound the candidate to the month, and addMonths() never yields a month past the end. Now the last supported day is mid-month, so a candidate on September 14th to 30th, 275760 goes through the validating constructor and throws, where master returned no occurrence. Probed: { freq: 'MONTHLY' } from +275760-08-20 over 09-01..09-13 throws "year 275760, month 9, day 20"; 3SU from 08-17 throws on the 21st; -1SU from 08-31 on the 28th; { freq: 'YEARLY' } from +275759-09-20 throws; countOccurrencesBefore(+275760-09-13) throws for those series. From 08-13 it correctly returns ['+275760-09-13']. The moved "should end every frequency on September 13, 275760" test only uses days ≤ 13 or ordinals that land in August, so it passes while this throws.

Failure scenario: a monthly series on the 20th that reaches the last month of the range crashes the view instead of ending, the same class round 1 reported for the 9999 edge.

Fix: addDays(month, this.monthlyTargetDay - 1) and addDays(monthStart, day - 1), which return null past LAST_SUPPORTED_DAY like the WEEKLY branch (findFirstInWeek, :241, already tests candidate !== null at :264). Add a MONTHLY day-20 case and a 3SU case expecting [] to the 275760 test.

3. ℹ️ Four spots throw at the start of the range, not two

Location: RecurringSeries.ts:507, :105, :195, :386

        plainDateFromDayNumber(period * SKIPPED_DAYS_PERIOD),

The reply lists the WEEKLY week start and hasOccurrenceBefore() "on the first day of Date". Probed in UTC: isSkippedDay()'s first 31-day period starts at day −100,000,017 (-271821-04-03), so any frequency with an occurrence in the first 14 days throws ("month 4, day 3"); getStartOfMonth() lands on -271821-04-01 for the whole of April; the WEEKLY week start reaches 04-19 for starts up to 04-25; hasOccurrenceBefore(FIRST) throws where master returned false. Only reachable from start: '-271821-04-20T00:00:00Z'-class input through the events prop (the dialog's regex is four-digit), so a note, but the description's "the first day" undercounts.

Fix: Math.max(period * SKIPPED_DAYS_PERIOD, -LAST_SUPPORTED_DAY) next to the existing Math.min at :508, and either clamp the month/week starts the same way or say "the first two weeks" in the description.

4. ℹ️ "The last 31-day period stops at that day" holds only in UTC

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/timeZone.ts:91

  if (offsetAtDayStart !== readOffset(offsets.format, (day + 1) * MS_PER_DAY - 1)) {

In any non-UTC series timezone, getSkippedDays() for the last period ends in getStartMs(LAST_SUPPORTED_DAY) → localToEpochMs → getAdapterOffsetMs(timezone, localMs + MS_PER_DAY) (:165), and Intl.DateTimeFormat throws a raw RangeError: Invalid time value past 8.64e15 ms. DAILY in Tokyo visiting any day from +275760-08-30 throws; toPlainDate(8.64e15, 'Asia/Tokyo'), createEndOfPlainDate(LAST - 1, 'Asia/Tokyo') and processEvent with an end at the last instant in Tokyo throw too. I checked master: it threw on all of these as well (its timeZone.ts is the same code), so nothing newly reachable, and the moved tests pass because they run in UTC (seriesStart(adapter.date(start, 'UTC'))). What is new is the inconsistency: the copy's getOffset() now clamps to ±MAX_EPOCH_MILLISECONDS (temporal-utils/zonedDateTime.ts:73) and ZonedDateTime(8.64e15 ns, 'Asia/Tokyo') works, while the Scheduler's own offset reader doesn't.

Fix: the same clamp in getTimezoneOffsetMs(), and run the "last supported day" tests in Asia/Tokyo too so the Math.min is actually exercised.

5. ℹ️ The first instant of Date west of UTC newly throws

Location: plainDate.ts:30, via toPlainDate() in timeZone.ts

toLocalDayNumber(-8.64e15, 'Pacific/Honolulu') is −100,000,001, one day before FIRST_SUPPORTED_DAY; the validating constructor throws "year -271821, month 4, day 19" where master's unvalidated createPlainDate returned -271821-04-19. Same asymmetry as the copy's ZonedDateTime (noted on #24040; the source's design, Temporal accepts it). Reachable only with the exact minimum Date in a negative-offset zone.

Fix: clamp the result of toLocalDayNumber() to the supported days in toPlainDate(), or one clause in the description.

6. ℹ️ Signed six-digit years break two string paths downstream

Location: packages/x-scheduler/src/internals/components/event-dialog/utils.ts:185 and packages/x-scheduler-internals-premium/src/internals/utils/recurring-events/rRuleString.ts:341

  return { date: isoString.slice(0, 10), time: isoString.slice(11, 16) };
    const untilIso = `${isoString.slice(0, 19).replace(/[-:]/g, '')}Z`;

Both slice at fixed offsets. For the end of 9999-12-31T23:30Z in Tokyo, the dialog's formatDateAndTimeFields() gives { date: '+010000-01', time: '01T08' }, and DATE_VALUE_REGEX (:143, four-digit) then rejects the date field. serializeRRule() writes UNTIL=+0100000101T0459Z for createEndOfPlainDate(9999-12-31, 'America/New_York') (which now returns +010000-01-01T04:59:59.999Z; master threw at .add({ days: 1 })), and parseRRule() rejects it. The dialog case is pre-existing (master produced the same string); the RRULE case is newly reachable but only isSameRRule() calls serializeRRule() outside tests, so the broken string is compared, not persisted. Flagged because the PR now pins +010000-01-01 as a supported value.

Fix: split on 'T' in the dialog and accept [+-]\d{6} in DATE_VALUE_REGEX; in serializeRRule() build the UNTIL from UTC fields with padStart(4) and reject or fall back to the instant form outside 0–9999, or make UNTIL_UTC_REGEX (:19) symmetric.

7. ℹ️ A fractional day number is cached as-is

Location: plainDate.ts:30-33

    const isoDate = dayNumberToIsoDate(dayNumber);
    plainDate = new BaseTemporal.PlainDate(isoDate.year, isoDate.month, isoDate.day);
    plainDateLookup.set(dayNumber, plainDate);
    dayNumberLookup.set(plainDate, dayNumber);

plainDateFromDayNumber(20000.5) prints 2024-10-04 but getDayNumber() of it returns 20000.5, so toEpochMs(..., 9h) lands at 21:00Z; master's createPlainDate() floored. No production caller passes a fraction today (parseRRule truncates intervals, the other 19 callers floor or are integer by construction), so this is a guard, not a bug: Math.floor(dayNumber) at the top, or a dev-mode integer assertion.

Tests (2)

1. 🟡 The Tokyo UNTIL/exDate test doesn't exercise its UNTIL or its exDate

Location: RecurringSeries.test.ts:122-123

      expect(days.map((day) => day.toString())).to.deep.equal(['2026-01-05', '2026-01-06']);
      expect(series.hasOccurrence()).to.equal(true);

The it() queries January 5th to 6th, 2026, so neither the until day nor the excluded day (both +010000-01-01) can change the result: I ran it without exDates, with timezone: 'UTC' (which is what a clamp to 9999-12-31 would look like) and even with the until on 2026-01-06, and the assertions pass every time. It is a "doesn't throw" test. The processing side is fine: processEvent.test.ts:53-54 asserts the +010000-01-01 key.

Fix: query the edge, probe-verified: with the exDate, getOccurrenceDays(P('9999-12-31'), P('+010000-01-02')) → ['9999-12-31']; without exDates → ['9999-12-31', '+010000-01-01']; with timezone: 'UTC' and no exDate → ['9999-12-31'] (with the exDate in UTC the last instant is still on 9999-12-31, so []).

2. ℹ️ The lower edge and the cache-clear contract aren't pinned

Location: plainDate.test.ts:22-25 and :50-61

The edge test asserts plainDateFromDayNumber(LAST_SUPPORTED_DAY) and + 1 only; -LAST_SUPPORTED_DAY → '-271821-04-20' and - 1 throwing are correct and untested (round 1 asked for both edges). The 10,000 clear is only hit as a side effect of the getPlainDateKey test; nothing asserts that a date read before the clear still equals() the one after and that getDayNumber() of the old instance still works, which is the contract the new JSDoc at :21-22 promises (both hold when probed). Two lines each. Also noting, no action: the copy's with() validation and the ESLint zone have no in-repo test; the zone does fire when probed with a bad import.

Simplifications (5)

1. 🟡 getWeekNumber() can be all day numbers

Location: packages/x-scheduler-internals/src/internals/utils/date-utils.ts:84

  const year = weekStartPlainDate.add({ days: 7 - minimalDays }).year;

Round 1's fix (isoDateToDayNumber(year, 1, minimalDays) on :86) brought the function from 459 to 277 ns bundled; this add() is the remaining fresh instance, 121 of those 277 ns (master: 213). Verified equal over 877,002 cases (years 1995–2035 plus full years −271820, −1, 0, 1, 9999, 10000, 275758, 275759 × week starts 1..7 × minimalDays 1..7), with the proposed form also returning values at the two range edges where the current one throws:

  checkWeekStartsOn(weekStartsOn);
  const weekStart = getStartOfWeekDayNumber(getDayNumber(plainDate), weekStartsOn);
  const { year } = dayNumberToIsoDate(weekStart + 7 - minimalDays);
  const firstWeekStart = getStartOfWeekDayNumber(isoDateToDayNumber(year, 1, minimalDays), weekStartsOn);
  return (weekStart - firstWeekStart) / 7 + 1;

Keep the explicit checkWeekStartsOn(): getPlainDateWeekStart() is where it runs today, and without it firstDay: 0 silently returns 40 instead of throwing.

2. ℹ️ The ESLint zone block can fold into the global rule, and it doesn't cover test/

Location: eslint.config.mjs:299-328

    files: [`packages/x-scheduler*/src/**/*${EXTENSION_TS}`],
    ignores: [`${BASE_TEMPORAL_COPY}/**`, `${BASE_TEMPORAL_COPY_INTERNALS}/**`],

The block exists only because flat config replaces rule options instead of merging them, hence the comment at :309 and the repeated zones. Verified alternative: the two base-temporal zones go into the global import/no-restricted-paths rule (:175-180), and the existing copy block (:287-298) gets 'import/no-restricted-paths': ['error', { zones: INTERNALS_INDEX_RESTRICTED_ZONES }] since the copy imports its own internals. 652 → 638 lines, no ignores, and the same five errors on the same bad imports as today (I ran both configs). Separately, the files glob leaves test/ out: a helper at test/utils/scheduler/x.ts importing @mui/x-scheduler-internals/internals/utils/base-temporal/PlainDate or .../iso lints clean (I checked with a real file). test/utils/addChaiAssertions.ts is exactly such a consumer (this PR had to move it off dayNumber), so add test/** to the zone's targets.

3. ℹ️ getSkippedDays() takes plain dates that every caller builds from day numbers

Location: timeZone.ts:289-301; callers getDayList.ts:24, RecurringSeries.ts:491, :507-508

      getSkippedDays(startDay, lastDay.add({ days: 1 }), this.timezone).size === 0

The function converts both arguments back to day numbers on entry; the callers do lastPlainDate.add({ days: 1 }), lastDay.add({ days: 1 }) and two plainDateFromDayNumber(...) only to feed it. No test calls it directly. With (fromDayNumber, endDayNumber, timezone) the three sites lose an allocation each, and :491 can no longer throw when lastDay is the last supported day, because no date is built.

4. ℹ️ The processed-date key goes through three caches to get back to its own number

Location: packages/x-scheduler-internals/src/internals/utils/plain-date-utils/adapter.ts:151 and timeZone.ts:151

    key: getPlainDateKey(plainDateFromDayNumber(dayNumber)),

Per processed occurrence: plainDateLookup (Map) → dayNumberLookup (WeakMap) → plainDateKeys (Map), to get a string keyed by the number the chain started from; toWallClockIsoString() formats the same string uncached. A getDayNumberKey(dayNumber) sharing the plainDateKeys cache (miss: plainDateFromDayNumber(dayNumber).toString(), so toString() stays the single formatter) serves both, and getPlainDateKey(plainDate) becomes getDayNumberKey(getDayNumber(plainDate)). That also settles the small inconsistency at plainDate.ts:46-49, where getDayNumber() handles non-ISO calendars that the Scheduler never creates while getPlainDateKey() documents that it ignores the calendar.

5. ℹ️ Three remaining build-a-date-to-read-a-field sites

Location: packages/x-scheduler-internals-premium/src/timeline-grid/header/iterate.ts:165 and :171; useEventOccurrencesWithDayGridPosition.ts:134

      return getDayNumber(plainDateFromLocalMs(localMs).with({ month: 1, day: 1 })) * MS_PER_DAY;

startOf('week') goes number → date → number → getStartOfWeekDayNumber → date → number (four lookups) when getStartOfWeekDayNumber(Math.floor(localMs / MS_PER_DAY), weekStartsOn) * MS_PER_DAY is the same value (verified over 640,200 cases); startOf('year') builds an uncached with() result and takes the cold getDayNumber() path, where a three-line getStartOfYearDayNumber() next to getStartOfMonthDayNumber would do. In the grid-position hook, getDayNumber(day.plainDate) is loop-invariant inside the per-occurrence loop. All sub-frame; listed because the description says this pattern was removed.

Docs (4)

1. ℹ️ "The interface of the namespace" describes the copied revision, not the current source

Location: PR description, "It's the interface of the namespace, instead of the PlainDate class."

True of the copy as it is in this branch. The source has since changed how the namespace declares its members, so after the re-sync the sentence will describe something slightly different; the migration itself doesn't care (nothing in the Scheduler references what changed, and with the newer copy applied TypeScript and the 4,054 tests pass). Worth rewording to "the type the namespace exposes" so the body survives the sync.

2. ℹ️ addMonths() says "last year", the code means "last day"

Location: RecurringSeries.ts:93-94

 * Returns the first day of the month `months` after the one starting on `monthStart`, or `null`
 * after the last year BaseTemporal supports.

:102 returns null when dayNumber > LAST_SUPPORTED_DAY, i.e. after the last day; with the range ending on September 13th, October 275760 is inside the last year yet returns null. addDays() at :84 already says "last day".

3. ℹ️ plainDateFromDayNumber() doesn't say it throws

Location: plainDate.ts:19-23

 * The plain date of a day number, in the ISO calendar.

This is the behaviour change round 1 was about (master's createPlainDate() returned an unvalidated date), and the guards at RecurringSeries.ts:89 and :102 exist because of it, but nothing at the definition states the contract. One line: "Throws, like the BaseTemporal.PlainDate constructor, outside the days it supports; check against LAST_SUPPORTED_DAY first when the day number can leave them."

4. ℹ️ Two body nits

Location: PR description, "No public change"

"SchedulerProcessedDay and EventCalendarFetchRange, the two models that hold plain dates" undercounts: the premium models/preset.ts (format(locale, plainDate), getStartPlainDate/getEndPlainDate, :70, :112-136) and SchedulerRecurringEventsPlugin.types.ts type them too. The conclusion holds (x-scheduler-premium/src/models/index.ts re-exports none of them), so "the models that hold plain dates (…) aren't re-exported" is enough. And "which only exports the namespace": base-temporal/index.ts also exports the field and option types (PlainDateFields, PlainDateFromOptions, ZonedDateTimeOptions, …), which the zone's except: ['./index.ts'] deliberately allows; drop "only".

Verdict

Approve after nits — the 9999 regression is fixed and verified end to end, and what remains is a cache cliff on a dataset nobody has yet, edge throws at a date nobody types, and a regression test that doesn't test its edge.


🤖 Review generated with Claude Code

@rita-codes rita-codes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment above.

flaviendelangle and others added 11 commits October 30, 2026 10:35
Type the plain dates as `BaseTemporal.PlainDate`, call the statics through
the namespace and read the day numbers through `getDayNumber()`, like the
Base UI version of BaseTemporal. The namespace files are copied as is, and
nothing is exported publicly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Leave the copied `zonedDateTime.ts` as it was.
- Read the day numbers once in the loops instead of on each iteration.
- Use `toLocalDayNumber()`, `getStartOfWeekDayNumber()`, `getStartOfMonthDayNumber()`,
  `addMonthsToDayNumber()` and `getDayCount()` instead of building a plain date to read its
  day number back.
- Compare the plain dates of the agenda with `BaseTemporal.PlainDate.compare()` and
  `equals()`, and step a day with `add()` outside the occurrence loops.
- Rename the imports where they were.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… occurrences

`getDaysTheOccurrenceIsVisibleOn()` read the day number of each visible day
for each occurrence. Through `getDayNumber()`, that's a call per day and per
occurrence where the field was a property read: the month view case of the
grouped-by-day bench was 6% slower. The caller now reads them once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keeps one `adapter.getTime()` reference in the series instead of two.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Scheduler only imports the `BaseTemporal` namespace from the index of
the copy, so it can move to the package of the library without the
internals of the copy:

- `plain-date-utils/dayNumber.ts` owns the ISO arithmetic of the day
  numbers: `isoDateToDayNumber()`, `dayNumberToIsoDate()`, the start of a
  week or a month, the addition of months, `MS_PER_DAY` and
  `LAST_SUPPORTED_DAY`.
- `getDayNumber()` reads the ISO fields of a plain date, and
  `plainDateFromDayNumber()` creates it with the `BaseTemporal.PlainDate`
  constructor. The plain dates of the day numbers and the day numbers of the
  plain dates are cached: the dates are immutable, and the views read the
  same days on every render.
- `getDayListFromPlainDates()` walks the day numbers, so its days come from
  the cache.
- `toWallClockIsoString()` formats the time with `BaseTemporal.PlainTime`.
- `PlainDateDuration` is the parameter of `BaseTemporal.PlainDate#add()`.
- An ESLint zone forbids the other modules of the copy outside of it.
- `toEqualPlainDate()` reads the public fields of a plain date.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`BaseTemporal.PlainTime` added the class to the bundles that don't use it,
like the Event Calendar.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… counts one

With an interval of millennia, the day before the first one fell before the
supported years, which the constructor of the plain dates rejects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… Scheduler

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The BaseTemporal copy supports the days of `Date`, up to September 13th,
275760, so `LAST_SUPPORTED_DAY` moves there. An instant at the end of 9999
east of UTC, such as a "no end date" UNTIL, exDate or event end in Tokyo, now
gives a date of the year 10000 instead of throwing.

- The last 31-day period of skipped days stops at the last supported day, as
  the day after it has no plain date.
- The ISO arithmetic is checked on every day of the years 0 to 9999, and on
  every 9973rd day of `Date`.
- The index of the copy exports the `WithFields` types, like the Base UI
  version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `getWeekNumber()` reads the day number of the first week from its ISO date,
  without building a plain date.
- The ESLint config names the two directories of the copy once, and its
  messages say what to import instead.
- The comments of the caches say how long their entries live, and that the
  plain dates of a day number are the same instance.
- A test checks that the wall-clock strings pad every part of the time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The monthly and yearly days after September 13th, 275760 return no occurrence instead of
  throwing, like the weekly ones.
- The Tokyo UNTIL and exDate test queries the last days of 9999 and 10000.
- `plainDateFromDayNumber()` says that it throws outside the supported days, and its tests cover
  the first supported day and the dates read before the cache is emptied.
- The Timeline header reads the start of a year with `getStartOfYearDayNumber()`, and the day grid
  reads the day number of its day once for all the occurrences.
- The ESLint zones of the BaseTemporal copy join the rule of all the files and cover `test/`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@flaviendelangle
flaviendelangle force-pushed the scheduler-base-temporal-namespace branch from fd85452 to c475d23 Compare October 30, 2026 09:44
@flaviendelangle

Copy link
Copy Markdown
Member Author

Thanks for the second round. c475d23 applies the items that held up. #24040 is merged, with your two clauses in its description, and this branch is rebased onto master.

Bugs

  1. 🟡 Cache thrash with long COUNT series: confirmed, not fixed here. Walking day numbers in countDays() and visitOccurrences() changes how RuleDays steps through a series, which is more than a review fix for this migration. A two-generation cache would keep the days of the view, but the walks would still miss most of their days. It stays under a frame on a dataset nobody has yet, so it's left for a follow-up.
  2. 🟡 MONTHLY and YEARLY after September 13th, 275760: fixed. Both candidates go through addDays(), which returns null past LAST_SUPPORTED_DAY, like the weekly branch. The 275760 test has a MONTHLY 20th case, a 3SU case and a YEARLY case. They threw before the fix.
  3. ℹ️ The start of the range: you're right, my reply undercounted. These are the first weeks of Date's range, which only the events prop can reach, so I didn't clamp them.
  4. ℹ️ Offsets past the last instant outside UTC: not changed. Master threw on the same inputs, as you checked, and they need the last instants of Date.
  5. ℹ️ The first instant of Date west of UTC: the description now says it throws, unlike Temporal. No clamp.
  6. ℹ️ Six-digit years in the dialog and in serializeRRule(): not changed. The dialog case comes from master, and the RRULE string is compared, never stored. Both need years outside 0 to 9999.
  7. ℹ️ Fractional day numbers: not changed, since no caller passes one.

Tests

  1. 🟡 Tokyo UNTIL and exDate: fixed. The test now queries December 31st, 9999 to January 2nd, 10000, with your three expectations: both days without the exDate, only 9999-12-31 with it, and only 9999-12-31 in UTC.
  2. ℹ️ Lower edge and cache clear: added. -LAST_SUPPORTED_DAY gives -271821-04-20, and the day before it throws. A date read before the cache is emptied equals() the new instance, and getDayNumber() still reads it.

Simplifications

  1. 🟡 getWeekNumber() in day numbers: not applied. Outside the hot loops, the Scheduler moves to plain dates, and a week number is read once per week.
  2. ℹ️ ESLint: applied. The zones of the copy join the rule of all the files, with test/ in their targets, and the copy keeps the zones of the internals index. With the same bad imports, the src ones still fail, and the two in test/ fail now.
  3. ℹ️ getSkippedDays() on day numbers: not applied, for the same reason as 1. It runs once per 31-day period of a series.
  4. ℹ️ getDayNumberKey(): not applied. It would be a second key helper next to getPlainDateKey().
  5. ℹ️ Dates built to read a field: startOf('year') uses a new getStartOfYearDayNumber(), like the month, and the day grid reads the day number of its day once. startOf('week') keeps getPlainDateWeekStart(), which checks weekStartsOn.

Docs

  1. Applied: "the type the namespace exposes".
  2. Applied: "last day".
  3. Applied: plainDateFromDayNumber() says that it throws outside the supported days.
  4. Applied both: the models that hold plain dates, and "only" is gone.

🤖 Comment generated with Claude

@flaviendelangle
flaviendelangle merged commit 9d698ee into mui:master Oct 30, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: scheduler Changes related to the scheduler. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants