fix(date-picker): eliminate infinite re-render crash on re-open with existing selection#4609
Conversation
…ng selection The useEffect that syncs picker state on open had initialStart and initialEnd — Date objects computed on every render — in its dependency array. Because Object.is returns false for any two distinct Date instances, the effect fired on every render when open=true, calling setRangeStart/setRangeEnd and triggering another render, producing an infinite loop that crashed the page. Fix: compute start and end as local variables inside the effect and use the stable string props (props.startDate, props.endDate) as deps instead. Also removes the redundant typeof fileSize === 'number' guard in the multipart quota check — fileSize is z.number() (required) in the contract so it can never be undefined at that point.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Adjusts multipart upload initiation to always run quota validation for non-exempt contexts and defensively passes Reviewed by Cursor Bugbot for commit 701d325. Configure here. |
Greptile SummaryThis PR fixes a crash in the date-picker caused by an infinite re-render loop, cleans up a dead-code type guard in the multipart upload route, and improves a JSDoc comment in
Confidence Score: 5/5Safe to merge — the fix is targeted and correct, the quota-guard cleanup is additive-safe, and no logic paths are broken. The root-cause fix (stabilising useEffect deps) directly addresses the crash. The multipart-route change only removes an unreachable type guard; quota enforcement itself is unchanged. No regressions were identified in the changed paths. No files require special attention, though callers supplying Date object instances rather than strings for startDate/endDate should ensure those references are stable. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Trigger as DatePicker Trigger
participant State as Component State
participant Effect as useEffect (sync on open)
User->>Trigger: Click (open false to true)
Trigger->>State: setInternalOpen(true)
Note over Effect: open=true, isRangeMode=true
Effect->>State: parseDate(startDate) sets rangeStart
Effect->>State: parseDate(endDate) sets rangeEnd
Effect->>State: setViewMonth / setViewYear
Note over Effect: Deps are all primitives - no re-fire on re-render
User->>Trigger: Click (open true to false)
Trigger->>State: setInternalOpen(false)
Note over Effect: open=false - early return, no state update
Reviews (2): Last reviewed commit: "refactor(date-picker): comprehensive cle..." | Re-trigger Greptile |
The previous fix still had derived Date objects in useEffect deps. Object.is(new Date(), new Date()) === false, so any Date in deps causes the effect to run every render, reproducing the infinite loop on re-open with existing time selection. Key changes: - useEffect deps now use only stable primitives (startDate, endDate strings) and compute Date values inside the effect — eliminating the loop - Replace `rest as any` with a FlatDatePickerProps merged type for safe, typed destructuring across the discriminated union - Remove initialStart/initialEnd render-scope variables; compute inline or inside effects to keep derivation local to each use site - Callbacks use destructured props (onChange, onRangeChange, etc.) instead of props.x references - Remove verbose TSDoc on internal callbacks — names are self-documenting - Preserve all existing JSX structure and CalendarMonth logic unchanged
|
@greptile |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 701d325. Configure here.
1 participant
Summary
Date-picker crash (root cause fix): The
useEffectthat syncs picker state on open hadinitialStartandinitialEnd—Dateobjects computed fresh on every render — in its dependency array. BecauseObject.isreturnsfalsefor any two distinctDateinstances, the effect re-fired on every render whileopen=true, callingsetRangeStart/setRangeEnd, triggering another render, and repeating until React's re-render limit was hit and the page crashed with "Something went wrong." Reproduced by: select a custom date range with time enabled, then click the picker again. Fix: computestart/endas local variables inside the effect; use the stable string props (props.startDate,props.endDate) as deps instead.Multipart quota guard cleanup: Removes the redundant
typeof fileSize === 'number'condition from the quota check.fileSizeis typed asnumber(required) ininitiateMultipartBodySchemaso the typeof guard was dead code. UsesfileSize ?? 0as a defensive fallback (the?? 0is unreachable but clarifies intent for future readers).JSDoc clarification: Improves the
QUOTA_EXEMPT_STORAGE_CONTEXTScomment to explain thatlogsis unreachable via the multipart endpoint (rejected at context validation) but active for non-multipart paths used by the execution logging pipeline.Test plan