Initial setup, the flow is there. #23
Conversation
… them, can see all and renders the page
|
@sid597 is attempting to deploy a commit to the Discourse Graphs Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Good start! Here's some initial feedback. It looks like the build script has some tight type validation which is failing currently. If you do For the type safety, I think we should use https://www.npmjs.com/package/zod. I believe we could set this up in such a way that when anyone adds an A bit of a nit, but I might want to move the We could change the terminology from "blogs" to "updates" ("Latest Blog Posts" -> "Latest Updates"), which would give a more open ended range until different patterns of updates emerge. I would have expected to see a Also, there are some |
|
Good catches Michael!
Fixed, thanks for the command I was not aware of filter, it did not show any errors locally, check is failing from deployment pov I think. Will the deployment happen once we merge or can we deploy before merge like a test branch build preview kinda thing?
Both Done
Yeah it was there I forgot to commit because I did
I use neovim which is currently not setup for typescript so did not have prettier or linting, will configure it now. |
I actually just renamed the packages after that message 😅
I believe the deployment normally auto updates, but it looks like it was still waiting on an authorization. We'll have to see how we can automate that. I authorized it and it deployed to
Ah interesting. We can look into this on Thursday. If we can't get prettier configured we can add an |
|
bug after mergin main to this branch, the error is same what you are mentioning in other pr. Looking into it if I can find anything |
Pretty cool, I like vercel+turbo+nextjs combo makes things easy. Will we be moving roam package to vercel as well? |
I agree but lets bring this up with the team as this was copied from a couple versions of previous sites. |
|
@sid597 Looking good for the initial setup. 🚀 After these changes, we could merge, then wait for the first "blog post" to be added and walk the PI's through it. ## Creating Blog Posts
1. [Fork](https://docs.github.com/en/github/getting-started-with-github/fork-a-repo) and [clone](https://docs.github.com/en/github/creating-cloning-and-archiving-repositories/cloning-a-repository) the repository.
2. [Create a new branch](https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop/managing-branches) for your update.
2. Copy `EXAMPLE.md` to create a new post
3. Rename the file to your desired URL slug (e.g., `my-new-post.md`)
4. Update the frontmatter and content
5. Set `published: true` when ready
6. Push your branch and open a pull request. 🚀 |
WalkthroughThis pull request introduces a comprehensive blog system for a Next.js website. It includes the creation of blog post pages, a blog index page, and integration of blog content on the home page. The implementation features a robust system for reading markdown files, parsing blog metadata, and rendering blog posts dynamically. Key functionalities include static generation, metadata handling, and type-safe validation with Zod, enhancing the overall structure and user experience of the blog application. Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
apps/website/tailwind.config.ts (1)
4-4: Ensure the type definition covers future expansions.Changing the type definition to
Pick<Config, "content" | "presets" | "plugins">is appropriate for supporting Tailwind plugins. When adding more configurations (e.g.,themeorvariants), ensure that they are either included in this type or that the file is updated to reduce the overhead of adding new properties in the future.apps/website/app/page.tsx (2)
15-36: Optimize or memoize repetitive file parsing if needed.
getLatestBlogs()repeatedly reads and parses files every time this component is rendered. If you anticipate large or frequently accessed data, consider an in-memory caching strategy or a build-time approach.
Line range hint
38-502: Optional: Format blog publication dates for better readability.
Currently, dates appear as raw strings (e.g.,2024-12-01). You could improve the user experience by formatting dates to something more user-friendly.- <p className="mt-2 text-sm italic text-gray-500"> - {blog.date} - </p> + <p className="mt-2 text-sm italic text-gray-500"> + {new Date(blog.date).toLocaleDateString("en-US", { + year: "numeric", + month: "long", + day: "numeric", + })} + </p>apps/website/app/blog/schema.tsx (1)
1-13: Optional: Validate dates as actual Date objects.
Right now,dateis validated asz.string(). If you need date-specific logic (like sorting or comparing), consider transforming it to aDatetype:export const BlogSchema = z.object({ - title: z.string(), - date: z.string(), - author: z.string(), + title: z.string(), + date: z.string().transform((val) => new Date(val)), + author: z.string(), });apps/website/app/blog/page.tsx (1)
7-23: No async operations detected; consider removing the async keyword.
getAllBlogs()is declared asasync, yet each call inside is synchronous (fs.readdirSync,fs.readFileSync). Consider removingasyncor switching to non-blocking I/O if you eventually need concurrency.- async function getAllBlogs(): Promise<Blog[]> { + function getAllBlogs(): Blog[] { // ... }apps/website/app/blog/[slug]/page.tsx (1)
36-40: Refine next.js route parameter type definition.
Havingparams: Promise<{ slug: string }>is unusual. Typically, Next.js route parameters are typed without a Promise. Consider removing thePromise<>wrapper for a more conventional approach.type Params = { - params: Promise<{ + params: { slug: string; - }>; + }; };apps/website/package.json (1)
14-14: Consider adding MDX-specific packagesThe current markdown processing setup uses
gray-matter,remark, andremark-html. However, since the PR objectives mention MDX document parsing, consider adding the following packages for proper MDX support:
@mdx-js/react@mdx-js/loaderAlso applies to: 18-19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
apps/website/app/blog/[slug]/page.tsx(1 hunks)apps/website/app/blog/page.tsx(1 hunks)apps/website/app/blog/schema.tsx(1 hunks)apps/website/app/layout.tsx(2 hunks)apps/website/app/page.tsx(2 hunks)apps/website/package.json(1 hunks)apps/website/tailwind.config.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/website/app/blog/[slug]/page.tsx
[error] 60-60: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (7)
apps/website/tailwind.config.ts (1)
10-10: Great addition for handling advanced typography.
The inclusion of @tailwindcss/typography is a best practice to leverage professional typographic defaults. This plugin enhances readability with minimal configuration overhead.
apps/website/app/page.tsx (1)
10-13: Consider using conditional file reads or graceful error handling.
Currently, the imports from fs, path, and gray-matter assume that app/blog/posts always exists. If your deployment environment or future changes affect this directory’s presence, you may want to implement checks or error handling where you read from the file system.
Would you like me to generate a shell script to verify the presence of app/blog/posts and ensure that your deployment supports file-based reads?
apps/website/app/blog/page.tsx (1)
25-63: Show a fallback message if no blog posts are found.
If blogs is empty, users won’t see any explicit message in the current implementation. A fallback display avoids confusion.
apps/website/app/layout.tsx (1)
Line range hint 3-72: Ensure added menu anchors map to real sections.
You’re linking to sections like /#updates and /#resources. Double-check that all these IDs exist on the home page to prevent broken in-page navigation. Everything else appears good!
apps/website/package.json (3)
20-20: LGTM: Type validation dependencies
The addition of zod and typescript aligns well with the PR objectives for implementing type safety, particularly for YAML entries in .md files.
Also applies to: 33-33
26-26: LGTM: Tailwind Typography plugin
The addition of @tailwindcss/typography is appropriate for styling markdown content in blog posts.
15-17: Review React RC versions for production readiness
While using ^15.0.3 for Next.js is fine, the React packages are using release candidate versions:
"react": "19.0.0-rc-66855b96-20241106"
"react-dom": "19.0.0-rc-66855b96-20241106"
Consider using stable versions for production deployments.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/website/app/blog/page.tsx (1)
43-81: 🛠️ Refactor suggestionAdd loading state and standardize date formatting.
The component needs the following improvements:
- Loading state handling for better UX
- Standardized date formatting
- Empty state handling (as previously suggested)
export default async function BlogIndex() { const blogs = await getAllBlogs(); + + const formatDate = (date: string) => { + return new Date(date).toLocaleDateString('en-US', { + year: 'numeric', + month: 'long', + day: 'numeric' + }); + }; + return ( <div className="flex-1 bg-gray-50"> <div className="mx-auto max-w-6xl space-y-12 px-6 py-12"> <div className="rounded-xl bg-white p-8 shadow-md"> <div className="mb-8"> <h1 className="text-4xl font-bold text-primary">All Updates</h1> </div> <div> <ul className="space-y-6"> + {blogs.length === 0 ? ( + <p className="py-8 text-center text-gray-500"> + No updates yet! Check back soon. 😊 + </p> + ) : ( {blogs.map((blog) => ( <li key={blog.slug} className="flex items-start justify-between border-b border-gray-200 pb-4 last:border-b-0" > <div className="w-4/5"> <Link href={`/blog/${blog.slug}`} className="block text-2xl font-semibold text-blue-600 hover:underline" > {blog.title} </Link> <p className="mt-2 text-sm italic text-gray-500"> - {blog.date} + {formatDate(blog.date)} </p> </div> <div className="w-1/5 text-right text-gray-600"> by {blog.author} </div> </li> - ))} + )))} </ul> </div> </div> </div> </div> ); }The empty state handling was previously suggested by mdroidian and is incorporated in the above changes.
🧹 Nitpick comments (2)
apps/website/app/blog/page.tsx (2)
7-8: Consider using a more robust path resolution strategy.Using
process.cwd()might lead to issues in production deployments, especially with serverless functions or Docker containers. Consider using Next.js's built-in path utilities or environment variables for better portability.-const BLOG_DIRECTORY = path.join(process.cwd(), "app/blog/posts"); +// Use Next.js app directory resolution +const BLOG_DIRECTORY = path.join(process.cwd(), "apps/website/app/blog/posts");
10-41: Consider enhancing error handling and adding performance optimizations.The function has good error handling but could benefit from the following improvements:
- More specific error logging
- Production performance optimizations
async function getAllBlogs(): Promise<Blog[]> { + // Add caching for production + if (process.env.NODE_ENV === 'production') { + // Check cache first + } + try { const files = await fs.readdir(BLOG_DIRECTORY); const blogs = await Promise.all( files .filter((filename) => filename.endsWith(".md")) .map(async (filename) => { const filePath = path.join(BLOG_DIRECTORY, filename); - const fileContent = await fs.readFile(filePath, "utf-8"); + const fileContent = await fs.readFile(filePath, "utf-8").catch(error => { + console.error(`Error reading file ${filename}:`, error); + return null; + }); + + if (!fileContent) return null; + const { data } = matter(fileContent); try { const validatedData = BlogSchema.parse(data); return { slug: filename.replace(".md", ""), ...validatedData, }; } catch (error) { - console.error(`Invalid front matter in file: ${filename}`, error); + console.error(`Invalid front matter in ${filename}:`, { + error, + data, + validationErrors: error.errors + }); return null; } }), ); return blogs.filter(Boolean) as Blog[]; } catch (error) { - console.error("Error reading blog directory:", error); + console.error(`Error reading blog directory ${BLOG_DIRECTORY}:`, error); return []; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/website/app/blog/page.tsx(1 hunks)
🔇 Additional comments (1)
apps/website/app/blog/page.tsx (1)
43-81: Verify the blog schema implementation.
Since this component relies on the BlogSchema from ./schema.tsx, let's verify its implementation.
✅ Verification successful
Let me generate additional scripts to verify the blog implementation and usage.
Let me try one more verification with adjusted commands.
Let me try one final verification with corrected commands.
Blog schema implementation is correctly used and validated
The blog schema implementation is properly integrated throughout the blog system:
- The
BlogSchemacorrectly defines required fields (title, date, author) using Zod validation - Schema validation is properly implemented in both:
getAllBlogs()function for the blog index pagegetBlog()function for individual blog posts
- Error handling is in place for invalid frontmatter data
- The schema is consistently used across all blog-related components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the blog schema implementation and its usage
# Find the schema file
echo "Searching for schema file..."
fd --type f --glob "schema.{ts,tsx}" --exec cat {}
# Find all blog post files to verify schema compliance
echo "\nListing all blog post files..."
fd --type f --glob "*.md" --exec head -n 10 {}
Length of output: 3916
Script:
#!/bin/bash
# Check how the schema is used and where blogs are loaded
echo "Checking blog loading implementation..."
ast-grep --pattern 'async function getAllBlogs'
echo "\nChecking blog frontmatter parsing..."
rg -A 10 "BlogSchema" --type ts --type tsx
echo "\nChecking for markdown files in blog directory..."
fd --type f --glob "*.md" --base-directory "apps/website/app/blog"
Length of output: 474
Script:
#!/bin/bash
# Check implementation with broader search
echo "Checking getAllBlogs implementation..."
rg -A 20 "getAllBlogs" --type typescript --type tsx
echo "\nChecking blog related files..."
fd . --type f --extension tsx --extension ts --extension md --base-directory "apps/website/app/blog"
echo "\nChecking BlogSchema usage..."
rg -A 10 "BlogSchema" --type typescript
Length of output: 531
Script:
#!/bin/bash
# Check the implementation of blog-related files
echo "Content of blog/page.tsx:"
cat "apps/website/app/blog/page.tsx"
echo "\nContent of blog/schema.tsx:"
cat "apps/website/app/blog/schema.tsx"
echo "\nContent of blog/[slug]/page.tsx:"
cat "apps/website/app/blog/[slug]/page.tsx"
Length of output: 5863
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/website/app/blog/[slug]/page.tsx (3)
10-32: Consider refining error handling for better user feedback.
Currently, any error encountered ingetBlogleads to anotFound()response. If you want to differentiate between a missing file vs. a frontmatter parsing error (or other types of errors), you could add more granularity in the catch block. This could improve debugging and user feedback.
69-81: Potential optimization for larger blogs.
For projects with numerous markdown files, repeated filesystem reads can become a bottleneck. Consider caching or indexing the list of blog posts if you anticipate a significant volume of content.
83-98: Expand metadata for improved SEO and social sharing.
Currently, metadata generation includes only the title and authors. Adding fields likedescription,keywords, orog:imagecan enhance discoverability and social media previews.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/website/app/blog/[slug]/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/website/app/blog/[slug]/page.tsx
[error] 58-58: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (2)
apps/website/app/blog/[slug]/page.tsx (2)
34-39: Verify the necessity of making 'params' a promise.
Typically, Next.js route parameters are available as a synchronous object. If there's no specific requirement to await params, consider reverting to a regular object type for clarity.
56-59: Guard against XSS by sanitizing HTML content.
This is a repeated concern from previous reviews. Ensure any untrusted or partially trusted input is sanitized before rendering.
- <article
- dangerouslySetInnerHTML={{ __html: contentHtml }}
- />
+ import DOMPurify from "dompurify";
+
+ <article
+ dangerouslySetInnerHTML={{
+ __html: DOMPurify.sanitize(contentHtml),
+ }}
+ />🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/website/app/layout.tsx (2)
24-24: Consider moving font configuration to a separate constants fileWhile the current implementation works, consider moving the font configuration to a separate constants file (e.g.,
constants/fonts.ts) to improve maintainability and reusability across the application.
51-72: Enhance navigation accessibility and user experienceConsider the following improvements:
- Add aria-label to the nav element
- Add active state styling for the current section
- Consider adding a mobile menu for better responsive design
- <nav className="w-full md:w-auto"> + <nav className="w-full md:w-auto" aria-label="Main navigation">Also, consider implementing active state detection:
// Add this helper function const isActive = (href: string) => { // Use Next.js usePathname hook to determine if the section is active const pathname = usePathname() return pathname === href } // Then in the Link className: className={`text-neutral-dark hover:text-neutral-dark/60 ${ isActive(`/#${item.toLowerCase()}`) ? 'font-bold' : '' }`}apps/website/app/page.tsx (1)
451-502: Enhance accessibility for the blog section.The implementation looks good but could benefit from some accessibility improvements:
Apply these enhancements:
<div className="space-y-6"> {blogs.length > 0 ? ( <> - <ul className="space-y-6"> + <ul className="space-y-6" role="list"> {blogs.map((blog) => ( <li key={blog.slug} className="flex items-start justify-between border-b border-gray-200 pb-4 last:border-b-0" > <div className="w-4/5"> <Link href={`/blog/${blog.slug}`} + aria-label={`Read ${blog.title}`} className="block text-2xl font-semibold text-blue-600 hover:underline" > {blog.title} </Link> - <p className="mt-2 text-sm italic text-gray-500"> + <time dateTime={blog.date} className="mt-2 block text-sm italic text-gray-500"> {blog.date} - </p> + </time> </div> <div className="w-1/5 text-right text-gray-600"> by {blog.author} </div> </li> ))} </ul> <div className="mt-6 text-center"> <Link href="/blog" + aria-label="View all blog updates" className="inline-block rounded-md bg-primary px-4 py-2 text-lg font-semibold text-white transition hover:text-white" > See All Updates → </Link> </div> </> ) : ( - <p className="text-left text-lg text-gray-600"> + <p className="text-left text-lg text-gray-600" role="status"> No updates yet! Check back soon. 😊 </p> )} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
apps/website/app/layout.tsx(3 hunks)apps/website/app/page.tsx(2 hunks)
🔇 Additional comments (3)
apps/website/app/layout.tsx (1)
4-6: LGTM: Appropriate imports added for navigation and styling
The new imports for Link, Image, and Inter font components are correctly added and necessary for the implemented functionality.
apps/website/app/page.tsx (2)
10-13: Consider Next.js build-time vs runtime implications.
The fs module usage should be carefully considered in Next.js components. While it works during build time, it won't work during runtime in edge or serverless environments. Consider using Next.js's built-in data fetching methods or moving this logic to a separate API route.
38-40: LGTM! Proper async implementation.
The conversion to an async component is correct and necessary for fetching blog data.
| <header className="flex flex-col items-center justify-between space-y-4 px-6 py-4 md:flex-row md:space-y-0"> | ||
| <Link href="/" className="flex items-center space-x-2"> | ||
| <Image | ||
| src="/logo-screenshot-48.png" | ||
| alt="Discourse Graphs Logo" | ||
| width={48} | ||
| height={48} | ||
| /> | ||
|
|
||
| <span className="text-3xl font-bold text-neutral-dark"> | ||
| Discourse Graphs | ||
| </span> | ||
| </Link> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add aria-label to improve logo link accessibility
The logo link should have an aria-label for better accessibility.
- <Link href="/" className="flex items-center space-x-2">
+ <Link href="/" className="flex items-center space-x-2" aria-label="Home">📝 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.
| <header className="flex flex-col items-center justify-between space-y-4 px-6 py-4 md:flex-row md:space-y-0"> | |
| <Link href="/" className="flex items-center space-x-2"> | |
| <Image | |
| src="/logo-screenshot-48.png" | |
| alt="Discourse Graphs Logo" | |
| width={48} | |
| height={48} | |
| /> | |
| <span className="text-3xl font-bold text-neutral-dark"> | |
| Discourse Graphs | |
| </span> | |
| </Link> | |
| <header className="flex flex-col items-center justify-between space-y-4 px-6 py-4 md:flex-row md:space-y-0"> | |
| <Link href="/" className="flex items-center space-x-2" aria-label="Home"> | |
| <Image | |
| src="/logo-screenshot-48.png" | |
| alt="Discourse Graphs Logo" | |
| width={48} | |
| height={48} | |
| /> | |
| <span className="text-3xl font-bold text-neutral-dark"> | |
| Discourse Graphs | |
| </span> | |
| </Link> |
| ].map((item) => ( | ||
| <li key={item}> | ||
| <Link | ||
| href={`/#${item.toLowerCase()}`} // Ensures absolute path with root `/` |
There was a problem hiding this comment.
💡 Codebase verification
Hash-based navigation implementation needs adjustments
The verification revealed both positive and concerning aspects:
- ✅ Smooth scrolling is properly implemented in
apps/website/app/globals.csswithscroll-smooth - ✅ Section IDs exist in
apps/website/app/page.tsxas confirmed by the ID pattern search - ❌ No fixed header or scroll offset implementation was found, which might affect the user experience when navigating to sections
Recommendations:
- Add a scroll margin top to section elements to prevent content from being hidden behind the header (if header is fixed)
- Consider implementing a fixed header with appropriate z-index and backdrop if not already present
🔗 Analysis chain
Verify hash-based navigation implementation
The current implementation uses hash-based navigation (/#section). Ensure that:
- Scroll behavior is smooth
- Sections are properly marked with corresponding IDs
- Consider implementing scroll offset to account for fixed header
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if sections with corresponding IDs exist
rg -l "id=[\"\'](?:about|resources|events|updates|talks|supporters|contact)[\"\']" --type html --type tsx
Length of output: 255
Script:
#!/bin/bash
# Check if sections with corresponding IDs exist, fixing the regex
rg -l "id=['\"](?:about|resources|events|updates|talks|supporters|contact)['\"]" --type html --type tsx
# Check for scroll behavior implementation
rg "scroll" --type css --type tsx -A 2
# Check for any scroll offset or header-related adjustments
rg "header.*fixed|position:\s*fixed" --type css --type tsx -A 2
Length of output: 298
Script:
#!/bin/bash
# Check if sections with corresponding IDs exist
rg -l "id=['\"](?:about|resources|events|updates|talks|supporters|contact)['\"]"
# Check for scroll behavior implementation
rg "scroll" -A 2
# Check for any scroll offset or header-related adjustments
rg "header.*fixed|position:\s*fixed" -A 2
# Look for any TypeScript/React files that might contain the sections
fd "\.(tsx|ts|jsx|js)$"
Length of output: 12941
apps/website/app/page.tsx
Outdated
| async function getLatestBlogs(): Promise<Blog[]> { | ||
| const blogDirectory = path.join(process.cwd(), "app/blog/posts"); | ||
| const files = fs.readdirSync(blogDirectory); | ||
|
|
||
| export default function Home() { | ||
| return ( | ||
| <div className={`min-h-screen bg-neutral-light ${inter.className}`}> | ||
| <header className="flex flex-col items-center justify-between space-y-4 px-6 py-4 md:flex-row md:space-y-0"> | ||
| <Link href="/" className="flex items-center space-x-2"> | ||
| <Image | ||
| src="/logo-screenshot-48.png" | ||
| alt="Discourse Graphs Logo" | ||
| width={48} | ||
| height={48} | ||
| /> | ||
| return files | ||
| .map((filename) => { | ||
| const filePath = path.join(blogDirectory, filename); | ||
| const fileContent = fs.readFileSync(filePath, "utf-8"); | ||
| const { data } = matter(fileContent); | ||
|
|
||
| <span className="text-3xl font-bold text-neutral-dark"> | ||
| Discourse Graphs | ||
| </span> | ||
| </Link> | ||
| <nav className="w-full md:w-auto"> | ||
| <ul className="flex flex-wrap justify-center space-x-4 md:flex-nowrap md:space-x-8"> | ||
| {[ | ||
| "About", | ||
| "Resources", | ||
| "Events", | ||
| "Talks", | ||
| "Supporters", | ||
| "Contact", | ||
| ].map((item) => ( | ||
| <li key={item}> | ||
| <Link | ||
| href={`#${item.toLowerCase()}`} | ||
| className="text-neutral-dark hover:text-neutral-dark/60" | ||
| > | ||
| {item} | ||
| </Link> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </nav> | ||
| </header> | ||
| const validatedData = BlogSchema.parse(data); | ||
|
|
||
| return { | ||
| slug: filename.replace(".md", ""), | ||
| ...validatedData, | ||
| }; | ||
| }) | ||
| .sort((a, b) => { | ||
| return new Date(b.date).getTime() - new Date(a.date).getTime(); | ||
| }) | ||
| .slice(0, 3); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and make the blog directory configurable.
The function needs improvements in two areas:
- Error handling for file operations and parsing
- Configuration for the blog directory path
Consider this improved implementation:
+const BLOG_DIRECTORY = process.env.BLOG_DIRECTORY || path.join(process.cwd(), "app/blog/posts");
+
async function getLatestBlogs(): Promise<Blog[]> {
- const blogDirectory = path.join(process.cwd(), "app/blog/posts");
- const files = fs.readdirSync(blogDirectory);
+ try {
+ const files = fs.readdirSync(BLOG_DIRECTORY);
- return files
- .map((filename) => {
- const filePath = path.join(blogDirectory, filename);
- const fileContent = fs.readFileSync(filePath, "utf-8");
- const { data } = matter(fileContent);
+ return files
+ .map((filename) => {
+ try {
+ const filePath = path.join(BLOG_DIRECTORY, filename);
+ const fileContent = fs.readFileSync(filePath, "utf-8");
+ const { data } = matter(fileContent);
- const validatedData = BlogSchema.parse(data);
+ const validatedData = BlogSchema.parse(data);
- return {
- slug: filename.replace(".md", ""),
- ...validatedData,
- };
- })
- .sort((a, b) => {
- return new Date(b.date).getTime() - new Date(a.date).getTime();
- })
- .slice(0, 3);
+ return {
+ slug: filename.replace(".md", ""),
+ ...validatedData,
+ };
+ } catch (error) {
+ console.error(`Error processing blog ${filename}:`, error);
+ return null;
+ }
+ })
+ .filter((blog): blog is Blog => blog !== null)
+ .sort((a, b) => {
+ return new Date(b.date).getTime() - new Date(a.date).getTime();
+ })
+ .slice(0, 3);
+ } catch (error) {
+ console.error("Error reading blog directory:", error);
+ return [];
+ }
}📝 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.
| async function getLatestBlogs(): Promise<Blog[]> { | |
| const blogDirectory = path.join(process.cwd(), "app/blog/posts"); | |
| const files = fs.readdirSync(blogDirectory); | |
| export default function Home() { | |
| return ( | |
| <div className={`min-h-screen bg-neutral-light ${inter.className}`}> | |
| <header className="flex flex-col items-center justify-between space-y-4 px-6 py-4 md:flex-row md:space-y-0"> | |
| <Link href="/" className="flex items-center space-x-2"> | |
| <Image | |
| src="/logo-screenshot-48.png" | |
| alt="Discourse Graphs Logo" | |
| width={48} | |
| height={48} | |
| /> | |
| return files | |
| .map((filename) => { | |
| const filePath = path.join(blogDirectory, filename); | |
| const fileContent = fs.readFileSync(filePath, "utf-8"); | |
| const { data } = matter(fileContent); | |
| <span className="text-3xl font-bold text-neutral-dark"> | |
| Discourse Graphs | |
| </span> | |
| </Link> | |
| <nav className="w-full md:w-auto"> | |
| <ul className="flex flex-wrap justify-center space-x-4 md:flex-nowrap md:space-x-8"> | |
| {[ | |
| "About", | |
| "Resources", | |
| "Events", | |
| "Talks", | |
| "Supporters", | |
| "Contact", | |
| ].map((item) => ( | |
| <li key={item}> | |
| <Link | |
| href={`#${item.toLowerCase()}`} | |
| className="text-neutral-dark hover:text-neutral-dark/60" | |
| > | |
| {item} | |
| </Link> | |
| </li> | |
| ))} | |
| </ul> | |
| </nav> | |
| </header> | |
| const validatedData = BlogSchema.parse(data); | |
| return { | |
| slug: filename.replace(".md", ""), | |
| ...validatedData, | |
| }; | |
| }) | |
| .sort((a, b) => { | |
| return new Date(b.date).getTime() - new Date(a.date).getTime(); | |
| }) | |
| .slice(0, 3); | |
| } | |
| const BLOG_DIRECTORY = process.env.BLOG_DIRECTORY || path.join(process.cwd(), "app/blog/posts"); | |
| async function getLatestBlogs(): Promise<Blog[]> { | |
| try { | |
| const files = fs.readdirSync(BLOG_DIRECTORY); | |
| return files | |
| .map((filename) => { | |
| try { | |
| const filePath = path.join(BLOG_DIRECTORY, filename); | |
| const fileContent = fs.readFileSync(filePath, "utf-8"); | |
| const { data } = matter(fileContent); | |
| const validatedData = BlogSchema.parse(data); | |
| return { | |
| slug: filename.replace(".md", ""), | |
| ...validatedData, | |
| }; | |
| } catch (error) { | |
| console.error(`Error processing blog ${filename}:`, error); | |
| return null; | |
| } | |
| }) | |
| .filter((blog): blog is Blog => blog !== null) | |
| .sort((a, b) => { | |
| return new Date(b.date).getTime() - new Date(a.date).getTime(); | |
| }) | |
| .slice(0, 3); | |
| } catch (error) { | |
| console.error("Error reading blog directory:", error); | |
| return []; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/website/app/blog/readBlogs.tsx (1)
9-10: Consider making the blog directory path configurable.
HardcodingBLOG_DIRECTORYcan limit flexibility. Exposing an environment variable or a configuration file can reduce future code changes if the directory location needs to vary.-const BLOG_DIRECTORY = path.join(process.cwd(), "app/blog/posts"); +const BLOG_DIRECTORY = process.env.BLOG_DIRECTORY + ? path.join(process.cwd(), process.env.BLOG_DIRECTORY) + : path.join(process.cwd(), "app/blog/posts");apps/website/app/blog/schema.tsx (1)
3-9: Consider stronger date validation.
Currently, thedatefield is a string, which may lack runtime safety. If you want to ensure valid date formats, consider usingz.coerce.date()orz.string().regex(...)with an appropriate pattern.export const BlogSchema = z.object({ title: z.string(), - date: z.string(), + date: z.coerce.date(), author: z.string(), published: z.boolean().default(false), });apps/website/app/blog/posts/EXAMPLE.md (5)
1-6: Enhance the example frontmatter valuesWhile the structure is good, consider making the example values more instructive:
--- -title: "Example Post - How to Create Blog Posts" -date: "2024-01-01" -author: "Devs" +title: "Your Blog Post Title Here" # Use a descriptive, engaging title +date: "2024-01-01" # Use today's date in YYYY-MM-DD format +author: "Your Name" # Use your full name or GitHub username published: false # Set to true to make this post visible ---
12-12: Fix heading hierarchyThe heading level jumps from H1 to H3. For proper document structure, use H2 instead.
-### Creating Blog Posts +## Creating Blog Posts🧰 Tools
🪛 Markdownlint (0.37.0)
12-12: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
14-21: Add local development instructionsConsider adding instructions for previewing changes locally before pushing:
5. Update the frontmatter and content 6. Set `published: true` when ready +6a. (Optional) Preview your changes locally: + ```bash + pnpm install + pnpm build + pnpm dev + ``` 7. Push your branch and open a pull request. 🚀
22-22: Fix heading hierarchy for consistencyKeep the heading hierarchy consistent with the previous section.
-### Required Frontmatter +## Required Frontmatter
24-33: Add validation rules for frontmatter fieldsConsider adding field validation rules to help contributors:
Every post must start with frontmatter in YAML format: ```yaml --- title: "Your Post Title" date: "YYYY-MM-DD" author: "Author's name" published: true # Set to true to make the post visible ---
+Field validation rules:
+-title: String, required, max 100 characters
+-date: String in YYYY-MM-DD format, required
+-author: String, required
+-published: Boolean, required</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ba6cc3d23a865fb8160c00bd811291f5bfec4fe5 and 400d1346043bf1b571c227cd7e6443f5fda59495. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `apps/website/app/blog/[slug]/page.tsx` (1 hunks) * `apps/website/app/blog/page.tsx` (1 hunks) * `apps/website/app/blog/posts/EXAMPLE.md` (1 hunks) * `apps/website/app/blog/readBlogs.tsx` (1 hunks) * `apps/website/app/blog/schema.tsx` (1 hunks) * `apps/website/app/page.tsx` (2 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * apps/website/app/blog/page.tsx </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 Biome (1.9.4)</summary> <details> <summary>apps/website/app/blog/[slug]/page.tsx</summary> [error] 31-31: Avoid passing content using the dangerouslySetInnerHTML prop. Setting content using code can expose users to cross-site scripting (XSS) attacks (lint/security/noDangerouslySetInnerHtml) </details> </details> <details> <summary>🪛 Markdownlint (0.37.0)</summary> <details> <summary>apps/website/app/blog/posts/EXAMPLE.md</summary> 12-12: Expected: h2; Actual: h3 Heading levels should only increment by one level at a time (MD001, heading-increment) </details> </details> </details> <details> <summary>🔇 Additional comments (10)</summary> <details> <summary>apps/website/app/blog/readBlogs.tsx (5)</summary> `11-19`: **Nice defensive check when the directory is missing.** This pattern of returning `false` rather than throwing an error can help gracefully handle the absence of the directory without interrupting execution. --- `21-36`: **Zod schema usage looks good.** The `processBlogFile` function properly catches and logs invalid frontmatter. This helps maintain data integrity without crashing the application. --- `43-58`: **Ensure unpublished blogs are properly excluded.** The filtering by `.published` means posts are hidden if `published` is false or missing. This aligns with the PR goal of only showing published content. --- `60-83`: **Return type consistency when blog is not published.** The `notFound()` short-circuits rendering. This is fine for draft or unpublished blogs. Just ensure you’re comfortable with a 404 rather than a custom page for unpublished content. --- `85-90`: **Efficient retrieval of latest blogs.** Sorting and slicing the array to return the top three items is clear and straightforward. </details> <details> <summary>apps/website/app/blog/[slug]/page.tsx (2)</summary> `31-31`: **Duplicate: Ensure you sanitize HTML content to guard against XSS.** You’re embedding HTML via `dangerouslySetInnerHTML`. Even though the content may come from trusted Markdown files, it’s safer to sanitize the output to avoid injection attacks. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Biome (1.9.4)</summary> [error] 31-31: Avoid passing content using the dangerouslySetInnerHTML prop. Setting content using code can expose users to cross-site scripting (XSS) attacks (lint/security/noDangerouslySetInnerHtml) </details> </details> --- `42-72`: **Static params and metadata generation are well-implemented.** This pattern ensures each blog is statically generated, improving performance and SEO. Logging in the catch block is a helpful debugging fallback. </details> <details> <summary>apps/website/app/page.tsx (2)</summary> `12-13`: **Async usage is appropriate.** Marking `Home` as `async` is consistent with your need to `await` data from `getLatestBlogs`. This helps ensure content is ready when rendered. --- `425-476`: **Good approach to toggling the latest updates display.** Your conditional rendering for `blogs.length > 0` ensures graceful fallback when no blog posts exist. The link to "See All Updates" is a clear call-to-action as well. </details> <details> <summary>apps/website/app/blog/posts/EXAMPLE.md (1)</summary> `8-10`: **LGTM!** The title and introduction are clear and effectively communicate the purpose of this template. </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
|
||
| This is a template post that shows how to create new blog posts. Keep this file as a reference and create new posts by following these instructions. | ||
|
|
||
| ### Creating Blog Posts |
There was a problem hiding this comment.
Good idea to put the instructions here 🔥
2 participants
The website extract the blog from the folder, shows 3 of them, can see all the blogs, when clicked takes to the blog and renders it.
Addresses #20
You can checkout the branch and build locally to test the functionality, ui & ux.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores