fix(sort): coerce NaN date times to epoch in sortByDate - #30
Conversation
…poison sort A truthy-but-unparseable date string yields NaN from getTime(), making the comparator return NaN and the resulting order unspecified. Coerce NaN to 0 so such projects degrade deterministically (epoch-equivalent) instead. Fixes #29.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthrough
ChangesDate sorting behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized sorting fix deterministically handles unparseable dates and includes focused tests; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThe PR prevents
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified. The comparator now maps invalid parsed timestamps to the intentionally documented epoch value, and the added tests cover ordering in both directions and equality stability.
|
| Filename | Overview |
|---|---|
| packages/core/src/lib/sortByDate.ts | Normalizes unparseable dates to epoch so the comparator always returns a deterministic numeric result. |
| packages/core/src/lib/tests/sortByDate.test.ts | Adds focused coverage for invalid-date placement in both sort directions and stable ordering when all dates are invalid. |
Reviews (1): Last reviewed commit: "fix(sort): coerce NaN date times to epoc..." | Re-trigger Greptile
Summary
"not-a-date") yieldsNaNfromgetTime(), so the comparator returnedNaNand the sort order was unspecified per the ECMAScript contract.NaNto0in the comparator (per the issue's proposal) so unparseable dates degrade deterministically to epoch instead of poisoning the sort.Fixes #29.
Notes for review
The issue was written against pre-#27 code where dateless resolved to
0; this branch uses-Infinityfor dateless. With the literal|| 0coercion, unparseable dates sort as epoch — older than every real date but newer than dateless. This matches the issue's "epoch-equivalent" wording.Verification
pnpm build && pnpm typecheck && pnpm lint && pnpm test— all green (1013 tests, incl. 2 new).Summary by CodeRabbit