Conversation
e5bfab5 to
9ea47e6
Compare
9ea47e6 to
7b7a6c1
Compare
| <Card> | ||
| <CardHeader> | ||
| <CardTitle>{module.title}</CardTitle> | ||
| <CardDescription>Card Description</CardDescription> |
There was a problem hiding this comment.
| <CardDescription>Card Description</CardDescription> | |
| <CardDescription>{module.description}</CardDescription> |
| } | ||
|
|
||
| export default function ModuleCard({ course, module }: Props) { | ||
| const coursePageUrl = `/${course.sourceLanguage.code}/courses/${course.targetLanguage.code}/courses/${module.slug}` |
There was a problem hiding this comment.
🐛 Bug: Potential Runtime Crash in ModuleCard Component
Issue: The URL building logic in ModuleCard can crash if course or module objects have null/undefined nested properties.
Current problematic code:
const coursePageUrl = `/${course.sourceLanguage.code}/courses/${course.targetLanguage.code}/courses/${module.slug}`
If course or module is null or undefined, then the app might crash | import { CourseDetail, Module } from '@/data/course' | ||
| import Link from 'next/link' | ||
|
|
||
| type Props = { |
There was a problem hiding this comment.
Issue: Generic Props type should be renamed to ModuleCardProps for better searchability, IDE support, and maintainability. Follows industry standards and makes refactoring easier.
// ❌ Current
type Props = { course: CourseDetail; module: Module }
// ✅ Suggested
type ModuleCardProps = { course: CourseDetail; module: Module }| <ul className="flex space-y-6 flex-col p-6"> | ||
| {detail.modules.map((module) => ( | ||
| <li key={module.slug}> | ||
| <ModuleCard course={detail} module={module} /> |
There was a problem hiding this comment.
Suggestion: Pass coursePageUrl from parent instead of building it inside the component. Keeps the card as a pure display component without business logic.
// ✅ Better - Pure display component
type ModuleCardProps = {
course: CourseDetail
module: Module
coursePageUrl: string // Parent handles URL logic
}| @@ -0,0 +1,5 @@ | |||
| import path from 'node:path' | |||
|
|
|||
| export function getAbsoluteCoursePath(jsonPath: string) { | |||
There was a problem hiding this comment.
Suggestion: Use default export since the file contains only a single function.
// ✅ Better - Default export for single function module
export default function getAbsoluteCoursePath(jsonPath: string) {
return path.join(process.cwd(), 'src', 'courses', jsonPath)
}
No description provided.