[scheduler] Validate missing and duplicate event ids#22790
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@rita-codes @flaviendelangle Kindly have a review on this pr whenever you get a chance. Thanks! |
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
179a858 to
fb444a9
Compare
Resolved. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
fb444a9 to
f8c8163
Compare
rita-codes
left a comment
There was a problem hiding this comment.
Thanks for working on this @Anexus5919! The implementation looks great! left a couple of minor style nits below.
The rest is perfect 🚢
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…ore/SchedulerStore.utils.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/tests/event.SchedulerStore.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/tests/event.SchedulerStore.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
Resolved. |
Signed-off-by: Adarsh Singh <[email protected]> Co-authored-by: Rita <[email protected]>
Closes #22759
Summary
buildEventsStateinserted events into the id lookups without validating the resolvedid, so duplicate or missing ids silently corrupted the scheduler state with no feedback:eventModelLookupandprocessedEventLookup, buteventIdListkept both copies. The array and the lookups disagreed, which surfaced as duplicate React keys (occurrences are keyed byString(event.id)) and events rendering with another event's data.idwasundefined, every id-less event collided on the same key, so only one survived. Silent data loss.The premium lazy-loading cache had the same gap: its default
getIdreturnsevent.id, andupsertkeyed entries byString(this.getId(event)), so id-less events all collapsed onto the literal"undefined"key and could be wrongly evicted as server-side deletes.SchedulerEvent.idis typed as required, so TypeScript users are nudged at compile time. The real victims are JavaScript users, dynamically sourced data, and thedataSourcepath where ids resolve at runtime.Changes
checkSchedulerEventIdIsValid(id, event)inSchedulerStore.utils.ts, mirroring the Data Grid'scheckGridRowIdIsValid. It throws a clearMUI X Schedulererror (what happened, why it is a problem, how to fix it) when the id isnull/undefined, including the offending event and pointing toeventModelStructure.idGetteras the per-event id source, or remains a valid id.buildEventsState. On a missing id it throws. On a duplicate id it emits a dev-onlywarnOnceand skips re-pushing the id, soeventIdListstays consistent with the last-wins lookups. This removes the list/lookup mismatch and the duplicate React keys in production, not just in development.upsert, validating the resolved id before stringifying it. BecausesetRangefunnels every event throughupsert, this also fixes the wrong eviction of id-less events.This keeps last-wins semantics for duplicates, matching the Data Grid's lenient direction, while fixing the scheduler-specific corruption that was worse than the Grid's (the Grid never diverged its id list from its lookup; the scheduler did).
Behavior
MUI X Scheduler: All events must have a unique \id`...` (registered as error code 287).Testing
event.SchedulerStore.test.ts: a missing id throws; two events sharing an id warn in dev, dedupe the id list, and keep the last event (verified acrossEventCalendarStore,EventCalendarPremiumStore, andEventTimelinePremiumStore).dataSource.test.ts:upsertandsetRangethrow on a missing id instead of colliding on"undefined".pnpm extract-error-codes(added code 287), typecheck,eslint, andprettier.x-scheduler-internalsandx-scheduler-internals-premiumpass (1235 passed, 3 skipped).How to reproduce
id(or adataSourcewhose getter cannot resolve one). Before this change one event silently disappears; after it throws a clear error.idand different titles. Before this change the event renders twice with duplicate React keys and mismatched data; after it renders once with a dev warning.