fix(sort): treat null dates as oldest in sortByDate - #27
Merged
Conversation
Null dates previously sorted to the TOP in desc order (and the bottom in asc), making dateless projects flip position with sort direction. Treat null as epoch-equivalent so dateless projects sort last in newest-first and first in oldest-first, consistently in both directions. This also matches the already-published docs for sortByDate. Fixes #23
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Address PR #27 review: clarify epoch-equivalent comment, collapse the mirrored desc/asc tests with it.each, and drop the redundant both-null test.
Type it.each rows via generic instead of repeated as-casts, assert stable input order in the all-dateless test, and restore ascending issue-number order of CHANGELOG link refs.
…-dates # Conflicts: # CHANGELOG.md
Greptile SummaryThe PR updates
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/core/src/lib/sortByDate.ts | Replaces direction-sensitive null branches with negative-infinity timestamps, correctly preserving null placement against all valid dates. |
| packages/core/src/lib/tests/sortByDate.test.ts | Adds focused regression coverage for both sort directions, omitted order, createdAt fallback, stable dateless ordering, and pre-1970 dates. |
| CHANGELOG.md | Adds the Unreleased public-API fix entry and issue reference. |
Reviews (2): Last reviewed commit: "fix(sort): keep dateless projects oldest..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The epoch-equivalent (0) sentinel sorted any real pre-1970 date as older than dateless projects, breaking the documented end/beginning placement. Use -Infinity so a missing date is older than any real date. NaN from (-Inf) - (-Inf) is treated as 0 by the sort spec, keeping the all-dateless case stable.
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.
Fixes #23
Problem
sortByDate(projects, 'desc')(the default "newest first") sorted projects with no date (updatedAtandcreatedAtboth null) to the top, ahead of every dated project. The same dateless row also flipped position with sort direction — top indesc, bottom inasc— because the null-comparison branches multiplied by the order multiplier.Change
packages/core/src/lib/sortByDate.ts— replaced the four null special-case branches with epoch-equivalent substitution: a missing date compares as0(oldest). Theasc/descmultiplier now applies uniformly, so dateless projects sort last in newest-first and first in oldest-first, consistently in both directions. Net effect: 4 branches → 2 lines.This also aligns the implementation with the already-published docs (
packages/docs/src/api/utilities/sort-by-date.md), which state: "Projects with no date are placed at the end (for'desc') or beginning (for'asc')".Tests
Updated the two tests that encoded the old behavior (dateless now asserted at end in
desc, beginning inasc) and added:desc)createdAt-only projects indesc(guards the fallback chain)createdAt-only projects inasc(mirror case)Review follow-up: the mirrored desc/asc pairs are collapsed into
it.eachcases, and a redundant both-null test was dropped (999 → 998 tests, same behavioural coverage).Changelog
Added an
[Unreleased]→ Fixed entry with a reference link to #23, per the public-API documentation requirement.Out of scope
nullsFirst/nullsLastoption — kept simple per the issue proposalVerification
pnpm build && pnpm typecheck && pnpm lint && pnpm test— all pass (998 tests, 61 files).