-
Notifications
You must be signed in to change notification settings - Fork 29
replace blog-data.json with direct markdown file reading #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis set of changes migrates blog post data management from a static JSON file to a server-side approach using markdown files and a JSON index. The blog page now fetches post data server-side, the pagination component supports both client and server navigation, and the markdown renderer is enhanced with richer features and improved styling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NextPage (Server)
participant BlogServer
participant FS (File System)
participant Index (articles-index.json)
User->>NextPage: Request /a/[slug]
NextPage->>BlogServer: getPostBySlugServer(slug)
BlogServer->>Index: Read metadata for slug
alt Markdown file exists
BlogServer->>FS: Read markdown file for slug
BlogServer->>NextPage: Return metadata + content
else Markdown missing
BlogServer->>NextPage: Return metadata + placeholder content
end
NextPage->>User: Rendered blog post page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
lib/markdown-renderer.tsx (1)
127-127: Security concern with dangerouslySetInnerHTML.Using
dangerouslySetInnerHTMLwithout sanitization could lead to XSS vulnerabilities if markdown content contains malicious scripts.Consider using a sanitization library:
import DOMPurify from 'isomorphic-dompurify' // In the component return: return ( <div className="prose prose-lg max-w-none" dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(renderMarkdown(content)) }} /> )Or migrate to a proper markdown library like
react-markdownwithremarkplugins for better security and maintainability.
🧹 Nitpick comments (3)
lib/blog.ts (1)
22-22: Minor: Variable name could be more descriptive.Consider renaming
poststomappedPostsor similar to distinguish it from the final sorted result.- const posts: BlogPost[] = articlesIndex.articles.map(article => ({ + const mappedPosts: BlogPost[] = articlesIndex.articles.map(article => ({ ...article, content: `# ${article.title}\n\nThis is a placeholder content for ${article.title}. The full content would be loaded from the markdown file.` })) - const sortedPosts = posts.sort((a, b) => { + const sortedPosts = mappedPosts.sort((a, b) => {lib/markdown-renderer.tsx (1)
5-6: Consider making BASE_PATH configurable.Hard-coding the BASE_PATH as "/StableViewpoints" reduces portability. Consider using an environment variable for better flexibility.
-const BASE_PATH = process.env.NODE_ENV === "production" ? "/StableViewpoints" : "" +const BASE_PATH = process.env.NODE_ENV === "production" ? (process.env.NEXT_PUBLIC_BASE_PATH || "") : ""components/pagination.tsx (1)
42-92: Consider reducing code duplication between navigation modes.The two rendering modes share significant styling and structure. Consider extracting common components or using a more unified approach.
Extract button/link components:
const PaginationButton = ({ isActive, disabled, onClick, href, children }: { isActive?: boolean disabled?: boolean onClick?: () => void href?: string children: React.ReactNode }) => { const baseClasses = "px-4 py-2 transition-colors" const activeClasses = isActive ? "bg-gradient-to-r from-[#228B22] to-[#91A511] text-white shadow-md cursor-default" : "bg-white border border-[#228B22]/20 text-[#228B22] hover:bg-[#228B22]/10 cursor-pointer" if (href) { return <Link href={href} className={`${baseClasses} ${activeClasses}`}>{children}</Link> } return ( <button onClick={onClick} disabled={disabled} className={`${baseClasses} ${disabled ? 'bg-gray-100 text-gray-400 cursor-not-allowed' : activeClasses}`} > {children} </button> ) }Also applies to: 94-157
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/a/[slug]/page.tsx(1 hunks)components/pagination.tsx(2 hunks)lib/blog-data.json(0 hunks)lib/blog-server.ts(1 hunks)lib/blog.ts(2 hunks)lib/markdown-renderer.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- lib/blog-data.json
🔇 Additional comments (8)
lib/blog.ts (2)
1-1: LGTM! Clean migration to the new data source.The import change from the deleted
blog-data.jsonto the newarticles-index.jsonaligns with the PR objective to replace JSON with markdown file reading.
16-19: Good approach using placeholder content for client-side usage.The mapping logic correctly transforms the articles index into BlogPost objects with placeholder content. This maintains the client-side interface while avoiding the need to load full markdown content on the client.
app/a/[slug]/page.tsx (3)
1-1: LGTM! Correct migration to server-side rendering.The switch from client-side
getPostBySlugto server-sidegetPostBySlugServeraligns perfectly with the PR objective and maintains the same functionality while enabling server-side markdown processing.Also applies to: 13-13
7-8: Good direct usage of articles index for static generation.Reading slugs directly from
articles-index.jsonfor static generation is efficient and maintains consistency with the new data source approach.
12-12: No changes needed for awaitedprops.paramsusage.Our search shows only one dynamic route page (
app/a/[slug]/page.tsx), and it correctly usesawait props.params. No other dynamic routes were found, so the pattern is applied consistently.lib/markdown-renderer.tsx (2)
23-29: LGTM! Well-implemented code block support.The fenced code block and inline code implementations are correct and include appropriate styling. The regex patterns properly handle the different code syntax variations.
72-77: Good ordered list implementation.The ordered list support is well-implemented with proper regex matching and consistent styling that matches the unordered list approach.
components/pagination.tsx (1)
11-11: LGTM! Good interface design for dual-mode support.Making
onPageChangeoptional enables the component to support both client-side and server-side navigation patterns, which aligns well with the overall architecture changes in this PR.
| export interface BlogPost { | ||
| slug: string | ||
| title: string | ||
| author: string | ||
| date: string | ||
| image: string | ||
| excerpt: string | ||
| content: string | ||
| featured: boolean | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Eliminate interface duplication.
The BlogPost interface is duplicated between lib/blog.ts and lib/blog-server.ts. This violates the DRY principle and could lead to inconsistencies.
Create a shared types file and import the interface in both modules:
// lib/types.ts
export interface BlogPost {
slug: string
title: string
author: string
date: string
image: string
excerpt: string
content: string
featured: boolean
}Then update both files to import from the shared location.
🤖 Prompt for AI Agents
In lib/blog-server.ts lines 6 to 15, the BlogPost interface is duplicated from
lib/blog.ts, violating DRY principles. To fix this, create a new file
lib/types.ts containing the BlogPost interface definition, then remove the
interface from both lib/blog-server.ts and lib/blog.ts, and import BlogPost from
lib/types.ts in both files.
| export function getPostBySlugServer(slug: string): BlogPost | null { | ||
| try { | ||
| const markdownPath = join(process.cwd(), 'public/articles', `${slug}.md`) | ||
|
|
||
| if (existsSync(markdownPath)) { | ||
| const fileContents = readFileSync(markdownPath, 'utf8') | ||
| const { content } = matter(fileContents) | ||
|
|
||
| // Find the article metadata from the index | ||
| const articleMetadata = articlesIndex.articles.find(article => article.slug === slug) | ||
|
|
||
| if (!articleMetadata) { | ||
| return null | ||
| } | ||
|
|
||
| return { | ||
| ...articleMetadata, | ||
| content | ||
| } | ||
| } else { | ||
| // Fallback: return post with placeholder content | ||
| const articleMetadata = articlesIndex.articles.find(article => article.slug === slug) | ||
| if (!articleMetadata) { | ||
| return null | ||
| } | ||
|
|
||
| return { | ||
| ...articleMetadata, | ||
| content: `# ${articleMetadata.title}\n\nThis is a placeholder content for ${articleMetadata.title}. The full content would be loaded from the markdown file.` | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error(`Error reading markdown file for ${slug}:`, error) | ||
| return null | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add input validation and improve security.
The function lacks input validation and could be vulnerable to path traversal attacks.
Apply these security improvements:
export function getPostBySlugServer(slug: string): BlogPost | null {
+ // Validate slug to prevent path traversal
+ if (!slug || typeof slug !== 'string' || !/^[a-zA-Z0-9-_]+$/.test(slug)) {
+ return null
+ }
+
try {
const markdownPath = join(process.cwd(), 'public/articles', `${slug}.md`)Also consider sanitizing error messages to avoid information disclosure:
} catch (error) {
- console.error(`Error reading markdown file for ${slug}:`, error)
+ console.error(`Error reading markdown file for slug: ${slug}`)
return null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getPostBySlugServer(slug: string): BlogPost | null { | |
| try { | |
| const markdownPath = join(process.cwd(), 'public/articles', `${slug}.md`) | |
| if (existsSync(markdownPath)) { | |
| const fileContents = readFileSync(markdownPath, 'utf8') | |
| const { content } = matter(fileContents) | |
| // Find the article metadata from the index | |
| const articleMetadata = articlesIndex.articles.find(article => article.slug === slug) | |
| if (!articleMetadata) { | |
| return null | |
| } | |
| return { | |
| ...articleMetadata, | |
| content | |
| } | |
| } else { | |
| // Fallback: return post with placeholder content | |
| const articleMetadata = articlesIndex.articles.find(article => article.slug === slug) | |
| if (!articleMetadata) { | |
| return null | |
| } | |
| return { | |
| ...articleMetadata, | |
| content: `# ${articleMetadata.title}\n\nThis is a placeholder content for ${articleMetadata.title}. The full content would be loaded from the markdown file.` | |
| } | |
| } | |
| } catch (error) { | |
| console.error(`Error reading markdown file for ${slug}:`, error) | |
| return null | |
| } | |
| } | |
| export function getPostBySlugServer(slug: string): BlogPost | null { | |
| // Validate slug to prevent path traversal | |
| if (!slug || typeof slug !== 'string' || !/^[a-zA-Z0-9-_]+$/.test(slug)) { | |
| return null | |
| } | |
| try { | |
| const markdownPath = join(process.cwd(), 'public/articles', `${slug}.md`) | |
| if (existsSync(markdownPath)) { | |
| const fileContents = readFileSync(markdownPath, 'utf8') | |
| const { content } = matter(fileContents) | |
| // Find the article metadata from the index | |
| const articleMetadata = articlesIndex.articles.find(article => article.slug === slug) | |
| if (!articleMetadata) { | |
| return null | |
| } | |
| return { | |
| ...articleMetadata, | |
| content | |
| } | |
| } else { | |
| // Fallback: return post with placeholder content | |
| const articleMetadata = articlesIndex.articles.find(article => article.slug === slug) | |
| if (!articleMetadata) { | |
| return null | |
| } | |
| return { | |
| ...articleMetadata, | |
| content: `# ${articleMetadata.title}\n\nThis is a placeholder content for ${articleMetadata.title}. The full content would be loaded from the markdown file.` | |
| } | |
| } | |
| } catch (error) { | |
| console.error(`Error reading markdown file for slug: ${slug}`) | |
| return null | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/blog-server.ts from lines 18 to 53, the function getPostBySlugServer
lacks input validation, making it vulnerable to path traversal attacks. Add
validation to ensure the slug contains only safe characters (e.g., alphanumeric,
dashes, underscores) before using it to build the file path. Also, sanitize
error messages in the catch block to avoid exposing sensitive file system
details by logging a generic error message instead of the full error object.
Summary by CodeRabbit
New Features
Bug Fixes
Chores