fix(utils): return original input from date formatters on invalid dates - #6249
fix(utils): return original input from date formatters on invalid dates#6249Arunendra21 wants to merge 2 commits into
Conversation
formatCompactTimestamp and formatAbsoluteDate both build their output from Date getters. An unparseable string produces an Invalid Date whose getters return NaN rather than throwing, so formatCompactTimestamp's try/catch fallback never ran and it returned "NaN-NaN NaN:NaN", while formatAbsoluteDate returned the literal "Invalid Date". Both now check Number.isNaN(date.getTime()) up front and fall back to the original input string, which is what the existing catch was meant to do. Valid dates are unaffected. Updates the formatCompactTimestamp invalid-date test to assert the returned value instead of only its type, and adds a matching test for formatAbsoluteDate. Co-authored-by: eeshsaxena <eeshsaxena@gmail.com>
|
@Arunendra21 is attempting to deploy a commit to the Sim Team on Vercel. A member of the Team first needs to authorize it. |
PR SummaryLow Risk Overview Tests were tightened: Reviewed by Cursor Bugbot for commit ff72651. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThe PR makes two date formatters return their original input when parsing produces an invalid date.
Confidence Score: 4/5The PR appears safe to merge, with only a non-blocking documentation mismatch around the new fallback contract. The invalid-date guards and tests correctly implement the intended behavior; only the exported functions' return documentation remains inconsistent with that behavior. Files Needing Attention: packages/utils/src/formatting.ts
|
| Filename | Overview |
|---|---|
| packages/utils/src/formatting.ts | Adds correct invalid-date guards to both formatters, but their TSDoc still promises only fixed formatted output. |
| packages/utils/src/formatting.test.ts | Adds precise assertions that invalid inputs are returned unchanged and malformed NaN output is avoided. |
Reviews (1): Last reviewed commit: "fix(utils): return original input from d..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // "Invalid Date"; fall back to the original input instead. | ||
| if (Number.isNaN(date.getTime())) { | ||
| return dateString |
There was a problem hiding this comment.
Both exported functions now return the original input for invalid dates, but their TSDoc still promises a formatted date string. Documenting this fallback prevents consumers from incorrectly assuming that every result conforms to the advertised fixed date format.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good call. I have updated the TSDoc @returns for both formatAbsoluteDate and formatCompactTimestamp to state that an unparseable input is returned unchanged, so consumers do not assume every result matches the advertised format.
Note in the @returns of formatAbsoluteDate and formatCompactTimestamp that an unparseable input is returned unchanged, so callers do not assume every result matches the advertised date format.
1 participant
Summary
formatCompactTimestampandformatAbsoluteDateinpackages/utils/src/formatting.tsbuild their output fromDategetters. An unparseable string produces an Invalid Date whose getters returnNaNrather than throwing, soformatCompactTimestampreturned"NaN-NaN NaN:NaN"(itstry/catchfallback never ran, since nothing throws) andformatAbsoluteDatereturned the literal"Invalid Date".Both now check
Number.isNaN(date.getTime())up front and fall back to the original input string, which is what the existingcatchwas already meant to do. Valid dates are unaffected.Type of Change
Testing
Verified both functions against valid and invalid inputs. Updated the existing
formatCompactTimestampinvalid-date test to assert the returned value (it previously only checkedtypeof result === 'string', so the"NaN-NaN NaN:NaN"output slipped through), and added a matching invalid-date test forformatAbsoluteDate.Checklist