refactor: code quality and feature improvements - dedupe fetch, safe pagination, sitemap, SEO, homepage optimization#2
Conversation
…pagination, sitemap, generateStaticParams, constants extraction, homepage optimization
Reviewed the changes across all 5 files. Found 2 issues worth addressing:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| export async function generateMetadata({ params }: { params: Promise<{ slug: string }> }): Promise<Metadata> { | ||
| const { slug } = await params; | ||
| const article = await getArticle(slug); | ||
| const article = await getArticleBySlug(slug, { next: { revalidate: 3600 } }); |
There was a problem hiding this comment.
The old inline getArticle() returned null only on HTTP 404 and threw on other status codes (500, 502, etc.), letting the error propagate to Next.js error boundaries. getArticleBySlug catches all errors and returns null, so a server outage or 500 now silently renders a "not found" page instead of showing the error UI. If distinguishing "not found" from "server error" matters here, getArticleBySlug would need to re-throw non-404 errors or accept an option to do so.
Fix it with Roo Code or mention @roomote and request a fix.
| const sectionResults = await Promise.all( | ||
| allSlugs.map(async (slug) => { | ||
| const category = categories.find((cat) => cat.slug === slug); | ||
| if (!category) return { slug, name: slug, articles: [] as Article[] }; | ||
| const articles = await getArticlesByCategory(slug, { limit: HOMEPAGE_ARTICLES_PER_CATEGORY }).catch(() => [] as Article[]); | ||
| return { slug, name: category.name, articles }; | ||
| }) |
There was a problem hiding this comment.
The removed groupArticlesByCategory explicitly walked child categories recursively (via collectIds) and matched articles belonging to any descendant. The replacement getArticlesByCategory(slug, { limit: 4 }) hits /categories/${slug}/articles -- if that API endpoint only returns articles directly assigned to the parent category (not its children), articles nested under subcategories will silently disappear from the homepage. Worth verifying the API behavior here; if it doesn't recurse, the homepage sections will show fewer articles than before.
Fix it with Roo Code or mention @roomote and request a fix.
1 participant
Changes
Code Quality
A1 - Deduplicate article fetching logic
getArticle()inarticles/[slug]/page.tsxwith the sharedgetArticleBySlug()fromlib/api.tsA2 - Safe pagination metadata handling
res.pagination!non-null assertions with runtime guardsA3 - Extract hardcoded category slugs
src/lib/constants.tswithHOMEPAGE_LEFT_COLUMN_SLUGS,HOMEPAGE_RIGHT_COLUMN_SLUGS, andHOMEPAGE_ARTICLES_PER_CATEGORYFeatures
B1 - generateStaticParams for article SEO
generateStaticParams()toarticles/[slug]/page.tsxB5 - Sitemap
src/app/sitemap.tsroute handlerlastModified,changeFrequency, andpriorityvaluesB8 - Homepage over-fetching fix
limit: 4limit: 10(only needed for featured/trending)groupArticlesByCategoryfunction