fix: complete safeToDate adoption in Calendar.renderInputTime#6285
Open
GiorgiM000 wants to merge 1 commit into
Open
fix: complete safeToDate adoption in Calendar.renderInputTime#6285GiorgiM000 wants to merge 1 commit into
GiorgiM000 wants to merge 1 commit into
Conversation
Commit 149d547 introduced `safeToDate()` to prevent crashes when date props are passed as strings, and adopted it across `src/index.tsx` and `src/time.tsx`. The three sibling callsites in `Calendar.renderInputTime` were missed: `startDate`, `endDate`, and `selected` still flow through raw `new Date(...)` construction, which silently accepts ISO strings and other non-Date values that the rest of the codebase now rejects. For TS-correct usage (Date-typed props), behavior is unchanged -- `safeToDate(Date)` returns the same Date instance. For consumers that pass non-Date values via `as any` or untyped `JSON.parse`, `Calendar.renderInputTime` now matches the rest of the codebase: returns `null` (and the time input does not render) rather than relying on `new Date(string)`, which is locale- and runtime- dependent and inconsistent across browsers. No new tests added -- the existing `renderInputTime` tests in `src/test/timepicker_test.test.tsx` cover the valid-Date and the null/undefined branches that `safeToDate` preserves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the
safeToDate()sibling-sweep started in 149d547 ("fix: prevent crash when date props are passed as strings") by adopting the helper in the three remaining raw-new Date()callsites inCalendar.renderInputTime.Context
safeToDate(date: Date | null | undefined): Date | nullwas introduced to guard against runtime crashes from consumers that pass non-Date values (strings, numbers, plain objects) to props typed asDate. After 149d547 the codebase has 7 callsites that adopted the helper:src/index.tsx:809,:811,:1258,:1320src/time.tsx:144,:224,:225Three sibling callsites in
src/calendar.tsxwere missed::1193—startDateinselectsRangemode:1200—endDateinselectsRangemode:1233—selectedin single-date modeThis PR closes that gap.
Behavior change
<DatePicker selected={new Date()} />(typed Date)<DatePicker selected={undefined} /><DatePicker selected={"2025-01-01" as any} />(TS violation)<DatePicker selected={"garbage" as any} />isValidguard catches Invalid Date)The behavior change only affects consumers who already bypass the TypeScript type contract by passing non-Date values. The new behavior aligns
Calendar.renderInputTimewith the documentedsafeToDatepolicy from 149d547.Diff
safeToDatefrom./date_utilsalongside the existingisValidimport.propValue ? new Date(propValue) : undefinedconstructions withsafeToDate(propValue) ?? undefined.startTime && isValid(startTime) && Boolean(propValue)checks are kept as-is (defensive, harmless redundancy with the new guard).Test plan
yarn type-check— cleanyarn eslint— cleanyarn jest src/test/timepicker_test.test.tsx src/test/show_time_test.test.tsx— passing (existingrenderInputTimecoverage in these suites exercises both branches)yarn jest --cifull suite — same 8 pre-existing failures as onmain(week_picker_test,datepicker_test); zero regressions introduced by this change. Verified by running the same tests againstmainwith this branch stashed.