[scheduler] Validate the FREQ value in parseRRule

[rita-codes]

Fixes #22769

Problem

parseRRule didn't validate the FREQ value against the supported set. In the RRULE-string branch it only checked that FREQ was present, then cast it straight to the union — so an unsupported value like FREQ=HOURLY passed parsing and blew up later with a message disconnected from the actual input. The object-input branch validated nothing at all.

Fix

Validate freq against DAILY | WEEKLY | MONTHLY | YEARLY at parse time in both branches, throwing a clear MUI X Scheduler: error that names the offending value (what happened / why / how to fix) instead of a late, disconnected failure.

The object branch is the typed API, so TS users are protected at compile time — but validating both at runtime is the defensive choice (JS users / dynamic data can bypass either).

Changes

• Added a validateFreq helper backed by a SUPPORTED_FREQUENCIES lookup table (Record<RecurringEventFrequency, true>, so it fails to compile if the union changes), used in both branches of parseRRule.
• Kept the object branch strict by design: a non-canonical freq (lowercase / whitespace) is a contract violation and is rejected rather than normalized, unlike the RRULE string which follows RFC 5545's case-insensitive parsing. Pinned with a test, plus a prototype-key regression test ({ freq: 'toString' }).
• Added a dedicated "FREQ is required" error for a nullish/empty freq on object input, matching the string branch.
• Added unit tests for the string branch (FREQ=HOURLY) and the object branch ({ freq: 'HOURLY' }).
• Ran pnpm extract-error-codes (new error codes 292 and 293).

[code-infra-dashboard]

Deploy preview

https://deploy-preview-22786--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-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]

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

[github-actions]

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

[github-actions]

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

[github-actions]

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

[flaviendelangle]

PR #22786 Review — [scheduler] Validate the FREQ value in parseRRule

Scope: 3 files, +37/−2. Adds a validateFreq helper + SUPPORTED_FREQUENCIES table to rRuleString.ts, wires it into both branches of parseRRule, adds 2 tests, and registers error code 292.

Overall: Solid, well-targeted fix that closes a real gap (previously FREQ=HOURLY was cast straight to the union and blew up later with a disconnected message). The error message follows the project's what/why/how format, the compile-time sync table is a nice touch, and hasOwnProperty.call is a deliberately correct defensive choice. No critical issues. The findings below are about consistency and polish.

Critical Issues (0)

None.

Important Issues (1)

1. String vs. object branch disagree on case/whitespace normalization — rRuleString.ts:46 vs :64-65
The string branch normalizes every value with .trim().toUpperCase() (line 65) before validating, so 'FREQ=daily' and 'FREQ= DAILY ' are accepted. The object branch calls validateFreq(input.freq) on the raw value, so { freq: 'daily' } or { freq: ' DAILY' } throws. Two logically-equivalent inputs behave differently.

Caveat on severity: the object API's freq is typed RecurringEventFrequency (uppercase-only union), so a correctly-typed TS user cannot hit this — it only bites untyped/JS callers or dynamic data, which is exactly the audience this PR's runtime check targets. So it's worth a deliberate decision rather than an automatic fix:

• Option A (parity): normalize in the object branch too and consume the return value — const freq = validateFreq(input.freq.toUpperCase()); then build the result with that freq.
• Option B (strict by design): keep rejecting non-canonical object input, but add a test that pins the decision so it reads as intentional.

Either way, add a test — both new tests use 'HOURLY' (invalid in both branches), so neither exercises this divergence.

Suggestions (5)

2. Object branch discards the narrowed return value — rRuleString.ts:46,53
validateFreq(input.freq) is called only for its throw side effect; the result type-narrowing is thrown away and correctness then leans on the as SchedulerProcessedEventRecurrenceRule cast (line 53). Functionally fine (the cast is not load-bearing since input.freq is already the union type), but consuming the return value would make the types — not call-ordering discipline — carry the guarantee. Resolves naturally if you take Option A above.

3. Missing/empty freq in the object branch gives a weaker message — rRuleString.ts:46 vs :86-92
The string branch has a dedicated "RRULE must include a FREQ property" error. In the object branch, { freq: '' } / a missing freq (untyped callers) falls through to Invalid FREQ value "" / "undefined" — it still throws (not silent — good), just with a less helpful diagnostic. Optional: emit a dedicated "freq is required" message for nullish input to match the string branch.

4. Add a prototype-key regression test — rRuleString.test.ts
The hasOwnProperty.call choice is correct and non-obvious — it's the only thing stopping { freq: 'toString' } / 'constructor' from validating as a real frequency. A future "simplify to freq in obj" refactor would silently break it. One test ({ freq: 'toString' } expects throw) cheaply protects this invariant.

5. Comment names a type that isn't on the line below it — rRuleString.ts:21
The comment says "Fails to compile if RecurringEventFrequency changes" but the code references SchedulerProcessedEventRecurrenceRule['freq']. Same type today (accurate), but anchoring the comment to the symbol actually used avoids silent drift if freq is ever retyped. Optionally consider ... = { ... } as const satisfies Record<RecurringEventFrequency, true> to freeze the table while keeping the same exhaustiveness check (catches both add and remove of union members).

6. PR description references the wrong error code
The description says "new error code 287", but the committed code correctly uses 292 (287 already exists for a different scheduler message). No code impact — just fix the description so it's not misleading on merge.

Out of scope (FYI, not for this PR)

silent-failure-hunter noted that one upstream lazy-loading path (SchedulerLazyLoadingPlugin.ts ~165-182, try/finally with no catch) would swallow this new throw as an unhandled rejection, while the fetch path (~217-221) surfaces it via pushError. Pre-existing infrastructure inconsistency, not introduced here — worth a separate issue if it matters.

Strengths

• Closes the real bug: validation now happens at the parse boundary for both input shapes, with a clear, actionable, correctly-prefixed error.
• Record<union, true> table genuinely fails compilation on union add/remove — keeps runtime in sync with the type, and the comment documents the intent.
• Object.prototype.hasOwnProperty.call correctly avoids prototype-key false positives.
• Error message text matches the runtime string and both test assertions exactly; error code is correctly sequenced.

Recommended action

1. Decide on finding #1 (normalize for parity, or keep strict) and add the corresponding test — this is the one substantive item.
2. Consider the cheap polish: prototype-key test (#4), and consume the narrowed value (#2) if you go with parity.
3. Fix the PR description's error-code number (#6).
4. Findings #3 and #5 are optional.

[rita-codes]

PR #22786 Review — [scheduler] Validate the FREQ value in parseRRule

Scope: 3 files, +37/−2. Adds a validateFreq helper + SUPPORTED_FREQUENCIES table to rRuleString.ts, wires it into both branches of parseRRule, adds 2 tests, and registers error code 292.

Overall: Solid, well-targeted fix that closes a real gap (previously FREQ=HOURLY was cast straight to the union and blew up later with a disconnected message). The error message follows the project's what/why/how format, the compile-time sync table is a nice touch, and hasOwnProperty.call is a deliberately correct defensive choice. No critical issues. The findings below are about consistency and polish.

Critical Issues (0)

None.

Important Issues (1)

1. String vs. object branch disagree on case/whitespace normalization — rRuleString.ts:46 vs :64-65 The string branch normalizes every value with .trim().toUpperCase() (line 65) before validating, so 'FREQ=daily' and 'FREQ= DAILY ' are accepted. The object branch calls validateFreq(input.freq) on the raw value, so { freq: 'daily' } or { freq: ' DAILY' } throws. Two logically-equivalent inputs behave differently.

Caveat on severity: the object API's freq is typed RecurringEventFrequency (uppercase-only union), so a correctly-typed TS user cannot hit this — it only bites untyped/JS callers or dynamic data, which is exactly the audience this PR's runtime check targets. So it's worth a deliberate decision rather than an automatic fix:

• Option A (parity): normalize in the object branch too and consume the return value — const freq = validateFreq(input.freq.toUpperCase()); then build the result with that freq.
• Option B (strict by design): keep rejecting non-canonical object input, but add a test that pins the decision so it reads as intentional.

Either way, add a test — both new tests use 'HOURLY' (invalid in both branches), so neither exercises this divergence.

Suggestions (5)

2. Object branch discards the narrowed return value — rRuleString.ts:46,53 validateFreq(input.freq) is called only for its throw side effect; the result type-narrowing is thrown away and correctness then leans on the as SchedulerProcessedEventRecurrenceRule cast (line 53). Functionally fine (the cast is not load-bearing since input.freq is already the union type), but consuming the return value would make the types — not call-ordering discipline — carry the guarantee. Resolves naturally if you take Option A above.

3. Missing/empty freq in the object branch gives a weaker message — rRuleString.ts:46 vs :86-92 The string branch has a dedicated "RRULE must include a FREQ property" error. In the object branch, { freq: '' } / a missing freq (untyped callers) falls through to Invalid FREQ value "" / "undefined" — it still throws (not silent — good), just with a less helpful diagnostic. Optional: emit a dedicated "freq is required" message for nullish input to match the string branch.

4. Add a prototype-key regression test — rRuleString.test.ts The hasOwnProperty.call choice is correct and non-obvious — it's the only thing stopping { freq: 'toString' } / 'constructor' from validating as a real frequency. A future "simplify to freq in obj" refactor would silently break it. One test ({ freq: 'toString' } expects throw) cheaply protects this invariant.

5. Comment names a type that isn't on the line below it — rRuleString.ts:21 The comment says "Fails to compile if RecurringEventFrequency changes" but the code references SchedulerProcessedEventRecurrenceRule['freq']. Same type today (accurate), but anchoring the comment to the symbol actually used avoids silent drift if freq is ever retyped. Optionally consider ... = { ... } as const satisfies Record<RecurringEventFrequency, true> to freeze the table while keeping the same exhaustiveness check (catches both add and remove of union members).

6. PR description references the wrong error code The description says "new error code 287", but the committed code correctly uses 292 (287 already exists for a different scheduler message). No code impact — just fix the description so it's not misleading on merge.

Out of scope (FYI, not for this PR)

silent-failure-hunter noted that one upstream lazy-loading path (SchedulerLazyLoadingPlugin.ts ~165-182, try/finally with no catch) would swallow this new throw as an unhandled rejection, while the fetch path (~217-221) surfaces it via pushError. Pre-existing infrastructure inconsistency, not introduced here — worth a separate issue if it matters.

Strengths

• Closes the real bug: validation now happens at the parse boundary for both input shapes, with a clear, actionable, correctly-prefixed error.
• Record<union, true> table genuinely fails compilation on union add/remove — keeps runtime in sync with the type, and the comment documents the intent.
• Object.prototype.hasOwnProperty.call correctly avoids prototype-key false positives.
• Error message text matches the runtime string and both test assertions exactly; error code is correctly sequenced.

Recommended action

1. Decide on finding Early feedback #1 (normalize for parity, or keep strict) and add the corresponding test — this is the one substantive item.
2. Consider the cheap polish: prototype-key test (Surface code review  #4), and consume the narrowed value (Color Accessibility threshold #2) if you go with parity.
3. Fix the PR description's error-code number (Discrepancy with main repository #6).
4. Findings Cells typography #3 and Accessibility, keyboard nav, major bug fixes #5 are optional.

Fixed! ✅

[flaviendelangle]

PR Review Summary — #22786 [scheduler] Validate the FREQ value in parseRRule

Verdict: Approve. No critical or blocking issues. The fix is real (not cosmetic), well-typed, well-tested, and follows the project's error-message conventions. The findings below are pre-existing gaps the PR deliberately scopes around, plus a few polish suggestions.

Critical Issues (0)

None. Every path the PR touches throws loudly with an actionable MUI X Scheduler: message; nothing is silently swallowed.

Important Issues (2 — both pre-existing, not introduced by this PR)

1. Object branch validates only freq, not the other fields — rRuleString.ts:26-34
   The object branch calls validateFreq(input.freq) then returns input as SchedulerProcessedEventRecurrenceRule, skipping the interval ≥ 1, byMonthDay 1–31, byMonth 1–12, count ≥ 1, and until-validity checks the string branch performs (lines 78–143). So { freq: 'DAILY', interval: -5 } or { ..., byMonthDay: [99] } still passes silently — the same bug class as #22769, just for object input. The public SchedulerEvent.rrule accepts objects (processEvent.ts:47 forwards them unsanitized), so this is reachable. Flagged by 3 of 5 agents. Pre-existing; the PR narrows the gap but leaves an inconsistent "only freq is validated" contract. Out of scope to fully fix here, but worth a follow-up issue or an explicit comment that other object fields are trusted.

2. validateFreq(freq: string) is reachable with non-string runtime values — rRuleString.ts:30
   Param is typed string, but JS callers can pass anything. Most non-strings throw correctly (1 → "1", {} → "[object Object]"), but a value that stringifies to a valid key (e.g. ['DAILY'] → "DAILY") would pass hasOwnProperty and propagate a wrong-typed value downstream. Low likelihood; a typeof freq !== 'string' guard at the top would close it and make the : string signature honest at the JS boundary.

Suggestions (4)

3. Duplicate "missing FREQ" messages — rRuleString.ts:66-72 vs the new empty-check in validateFreq (error code 293). The line-66 guard throws first for strings, making validateFreq's empty-check dead for the string branch (only fires for object {}). Result: two near-identical, zero-argument error messages for the same condition, chosen by input type. CLAUDE.md says to consolidate codes with identical semantics/arg-count — consider reusing one message or letting validateFreq own the case for both branches (folding in the string-specific guidance).

4. Comment #2 is mildly ambiguous — rRuleString.ts object branch: "validates freq as-is…" can be misread as "passes through without validation" (the opposite of intent). Suggest tightening to convey the why, e.g. "Object input must already be canonical (freq is typed as RecurringEventFrequency), so reject as-is rather than uppercasing like the free-form RRULE string below."

5. Test gaps (minor): object-branch happy path is only transitively covered (existing DAILY tests at lines 10–25); no direct test that WEEKLY/MONTHLY/YEARLY object input parses — so a regression dropping one from the allow-list wouldn't be caught on the object branch. Also FREQ= (empty value, string branch) is untested. The 5 prototype/casing/invalid-value tests are otherwise excellent and non-redundant.

6. Cosmetic: satisfies Record<RecurringEventFrequency, true> would give the identical exhaustiveness guard while preserving literal key types — purely stylistic, current form is correct.

Strengths

• The fix is genuine — freq: validateFreq(rruleObject.FREQ) (line 104) replaces the unchecked as cast that caused #22769; invalid FREQ now fails at the parse boundary with a message naming the bad value.
• Record<RecurringEventFrequency, true> is the standout — empirically verified (with repo tsc 5.9.3) to fail compilation in both directions if the union gains or loses a member, keeping the runtime allow-list in sync with the type. Strictly stronger than the file's existing Set<string> pattern, and the comment describing it is accurate.
• Prototype-safety done right — Object.prototype.hasOwnProperty.call(...) correctly rejects 'toString'/'constructor', pinned by a dedicated regression test.
• Reject-over-normalize for object-input casing is the correct anti-silent-failure posture, and is locked in by a test.
• Error messages follow the CLAUDE.md what/why/how format; error-codes.json (292, 293) matches the runtime strings exactly (extract-error-codes was run).

Recommended Action

1. None blocking — the PR is mergeable as-is.
2. Consider the typeof guard (#2) and the duplicate-message consolidation (#3) if you want to round out the error-handling story now.
3. File a follow-up for the object-branch field validation (#1) — it's the natural next step toward the PR's stated goal but legitimately out of this PR's scope.
4. Optional: tighten comment #2 (#4) and add the object-branch happy-path test (#5).