fix(blog): compare full ISO timestamps when sorting, and drop posts without a slug - #1654
Merged
Conversation
The date comparator built a Date by concatenating a time onto post.date, which assumes the value is exactly YYYY-MM-DD. BlogPost.date is a plain string and the CMS also stores full ISO timestamps, so "2026-08-05T12:51:59Z" became "2026-08-05T12:51:59ZT00:00:00" -> Invalid Date -> getTime() is NaN. A NaN comparator result is treated as +0 and toSorted is stable, so a post with a full timestamp never moved -- it stayed wherever the records put it. Listings with sortBy "date_desc" showed yesterday's post above today's. Only append a time when the value is a bare date, and use T00:00:00Z so a bare date is UTC midnight instead of local midnight -- ordering no longer depends on the server timezone. Unparseable values fall back to 0 so NaN never reaches the comparator. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Tagging OptionsShould a new tag be published when this PR is merged?
|
Contributor
📝 WalkthroughWalkthroughThis change updates blog post date parsing in ChangesBlog date sorting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A post whose slug is missing, empty or blank has no route. It still rendered in listings, producing cards that link nowhere and a broken url in the JSON-LD. Filter it out in filterPosts, ahead of every other filter and of slicePosts, so `count` still yields `count` renderable posts. Records come straight from the CMS and are cast without validation, so the guard also checks the type: a non-string slug would otherwise throw inside handlePosts, and the try/catch in the loaders would turn that into an empty listing. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Normalizing only bare YYYY-MM-DD left a gap: per spec a date-only string is parsed as UTC, but an ISO datetime with no timezone designator is parsed as local time. So "2026-08-05T23:30:00" ordered differently against a bare "2026-08-06" depending on the server timezone -- the two swap places in America/Sao_Paulo but not in UTC or Asia/Tokyo. Match any ISO date or datetime lacking a designator and append Z. Covers minute precision and fractional seconds too; strings that already carry a Z or a ±hh:mm offset are left untouched. Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Two fixes to
blog/core/handlePosts.ts.1. Date sorting ignored full ISO timestamps
The date branch of the
sortPostscomparator built aDateby concatenating a time onto whatever was inpost.date:That assumes every date is exactly
YYYY-MM-DD. ButBlogPost.dateis just astringand the CMS also stores full ISO timestamps. For"2026-08-05T12:51:59Z"the concatenation yields"2026-08-05T12:51:59ZT00:00:00"→Invalid Date→getTime()isNaN→ the comparison isNaN.Per spec (SortCompare: "If v is NaN, return +0") a
NaNcomparator result is treated as+0, andtoSortedis stable — so a post with a full ISO timestamp was never moved. It just stayed wherever the records happened to put it. Symptom:BlogpostListwithsortBy: "date_desc"listed yesterday's post above today's.Timezone dependence, in two rounds
The same expression made ordering depend on the server timezone, because
"2026-08-04T00:00:00"carries no timezone designator and is therefore parsed as local midnight.Fixing that for bare dates alone turned out to be insufficient (review thread): per spec a date-only string is parsed as UTC, but a datetime with no designator is parsed as local. So an offset-less
"2026-08-05T23:30:00"still ordered inconsistently against a bare"2026-08-06":Fix
Any ISO date or datetime lacking a timezone designator is pinned to UTC; strings that already carry a
Zor a±hh:mmoffset pass through untouched. The pattern covers minute precision and fractional seconds, not just whole seconds:The
|| 0keepsNaNfrom ever reaching the comparator, so an unparseable date sorts last fordesc— consistent with the existing!a[sortMethod]guards, which already push missing values to the end.Verified against the real
sortPosts, including with the input order flipped — since the old behavior depended on the order records arrived in, passing in both directions is what shows it is actually sorting rather than getting lucky:Every offset-less form resolves to the same instant as its explicit-UTC twin, and the day-boundary ordering above is stable across all five timezones tested.
2. Posts without a slug were rendered in listings
A post whose slug is missing, empty or blank has no route, so it can never be rendered — but it still appeared in listings, producing cards that link nowhere and a broken url in the JSON-LD.
filterRoutablePostsdrops them insidefilterPosts, ahead of every other filter and ofslicePosts, socountstill yieldscountrenderable posts.The guard checks the type as well as the value, which is load-bearing: records come from
getRecordsByPath, which casts the CMS JSON without validating it, soslugis a string only by convention. A non-string slug would makeslug.trim()throw insidehandlePosts— and thetry/catchinBlogpostListturns a throw intologger.error+return null, so one malformed record would blank out the entire listing instead of just dropping itself. Worse than the bug being fixed.Cases checked against the real
handlePosts(only the valid post survives, and an all-invalid input returnsnull, matching the existing empty-list contract):slugundefined""null" "123nullCompatibility
sortPostsandhandlePostsare unchanged, andSortBy/VALID_SORT_ORDERSare untouched.Two visible changes for live sites, both intended:
Worth a note in the release notes.
Checks
deno task check(the pre-commit hook) passes on every commit: fmt over 2099 files, lint over 1979, anddeno checkon everymod.ts.Left out on purpose
blog/sections/Template.tsx:74has the same string-concat pattern, but for display — with a full ISO timestamp it renders "Invalid Date". Same root cause, different symptom; kept out to keep this PR scoped.BlogPostPageandBlogPostItemboth doposts.find((post) => post.slug === slug). If the route param is everundefined, that matches a slug-less record; those loaders don't go throughhandlePosts, so this PR doesn't cover them.localeComparebranch has its inversion backwards for string fields (title_ascreturns Z→A). Pre-existing, out of scope, and sites are presumably compensating — but worth a separate issue.🤖 Generated with Claude Code