-
-
Notifications
You must be signed in to change notification settings - Fork 0
✨ add docs #371
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
base: master
Are you sure you want to change the base?
✨ add docs #371
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! 32 files out of 182 files are above the max files limit of 150. You can disable this status message by setting the 📝 WalkthroughWalkthroughA comprehensive documentation system is introduced with support for multiple projects (gitmoji, translations, next-sitemap), multi-language content in English and French, MDX processing with custom components, and dynamic routing via Next.js App Router. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant NextRouter as Next.js Router
participant DocsPage as DocsPage Component
participant MetadataLib as Metadata Library
participant MDXProcessor as MDX Processor
participant DocsSidebar as Sidebar/Components
participant Browser2 as Browser (Rendered)
Browser->>NextRouter: Request /docs/[project]/[...slug]
NextRouter->>DocsPage: Render with params
DocsPage->>MetadataLib: getDocsPageBySlug(slug, locale)
MetadataLib-->>DocsPage: DocsPage (markdown + frontmatter)
DocsPage->>MDXProcessor: processMDX(content, locale)
MDXProcessor->>MDXProcessor: Parse with remark/rehype plugins
MDXProcessor->>MDXProcessor: Extract headings & compute reading time
MDXProcessor-->>DocsPage: { content, headings, readingTime }
DocsPage->>MetadataLib: getDocsBreadcrumbs(slugs)
MetadataLib-->>DocsPage: Breadcrumb trail
DocsPage->>MetadataLib: getProjectMetadata(project)
MetadataLib-->>DocsPage: Sections & pages for sidebar
DocsPage->>DocsSidebar: Render with metadata
DocsSidebar->>DocsSidebar: Build nav tree, highlight active page
DocsPage->>DocsPage: Compose layout (Sidebar, Header, Content, ToC)
DocsPage-->>Browser2: Rendered HTML with all components
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Comment |
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: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
apps/web/src/content/fr/docs/gitmoji/getting-started.mdx-6-6 (1)
6-6: Fix missing French accents throughout the document.Multiple lines contain French words missing proper accents, which affects the professionalism and correctness of the documentation. Key corrections needed:
- Line 6: "inspiree" → "inspirée", "personnalisee" → "personnalisée"
- Line 10: "etre structure" → "être structuré"
- Line 16: "complete" → "complète"
- Line 43: "fonctionnalite" → "fonctionnalité"
- Line 45: "Ameliorer" → "Améliorer"
- Line 46: "a jour" → "à jour"
- Line 50: "Deplacer" → "Déplacer"
- Line 51: "a jour" → "à jour", "dependances" → "dépendances"
- Line 52: "Retrograder" → "Rétrograder", "dependances" → "dépendances"
- Line 55-56: "Ecrivez" → "Écrivez", "imperatif" → "impératif"
- Line 69: "Numero" → "Numéro"
- Line 71: "numero" → "numéro"
- Line 91: "representer" → "représenter", "melangez" → "mélangez", "lies" → "liés"
Based on learnings, this MDX content should be stored in the
content/directory for documentation pages.Also applies to: 10-10, 16-16, 43-43, 45-46, 50-52, 55-56, 69-69, 71-71, 91-91
apps/web/src/content/fr/docs/gitmoji/meta.json-16-16 (1)
16-16: Fix the French accent.Line 16 contains "complete" which should be "complète" in French.
🔎 Proposed fix
- "description": "Liste complete des gitmojis" + "description": "Liste complète des gitmojis"apps/web/src/content/fr/docs/gitmoji/with-ai.mdx-29-29 (1)
29-29: Fix missing accent in French text."creer" should be "créer" (with acute accent).
-Lancez la commande `/commit` pour creer un commit : +Lancez la commande `/commit` pour créer un commit :apps/web/src/content/fr/docs/gitmoji/with-ai.mdx-47-47 (1)
47-47: Fix missing accent in French text."preferez" should be "préférez" (with acute accent).
-Si vous preferez ne pas installer le plugin, ajoutez ceci au `CLAUDE.md` de votre projet : +Si vous préférez ne pas installer le plugin, ajoutez ceci au `CLAUDE.md` de votre projet :apps/web/src/content/fr/docs/gitmoji/with-ai.mdx-39-39 (1)
39-39: Fix missing accent in French text."creera" should be "créera" (with acute accent).
-Claude Code analysera vos changements et creera un commit suivant la convention gitmoji. +Claude Code analysera vos changements et créera un commit suivant la convention gitmoji.apps/web/src/content/fr/docs/gitmoji/with-ai.mdx-6-6 (1)
6-6: Fix missing accent in French text."generer" should be "générer" (with acute accent).
-Installez notre plugin gitmoji pour [Claude Code](https://claude.ai/code) pour generer des commits suivant la convention. +Installez notre plugin gitmoji pour [Claude Code](https://claude.ai/code) pour générer des commits suivant la convention.apps/web/src/content/fr/docs/gitmoji/with-ai.mdx-42-42 (1)
42-42: Fix missing accent in French text."recupere" should be "récupère" (with acute accents).
- La commande `/commit` recupere automatiquement votre git status et diff pour generer le message de commit approprie. + La commande `/commit` récupère automatiquement votre git status et diff pour générer le message de commit approprié.Note: This line also has "generer" → "générer" and "approprie" → "approprié".
apps/web/src/content/fr/docs/gitmoji/emojis.mdx-3-3 (1)
3-3: Fix missing French accents.The French text is missing diacritics in several places:
- Line 3:
complete→complète- Line 6:
categoriser→catégoriser,complete→complète- Line 11:
experience→expérience🔎 Proposed fix
--- title: Liste des emojis -description: Liste complete des gitmojis +description: Liste complète des gitmojis --- -Nous utilisons [gitmoji](https://gitmoji.dev) pour categoriser nos commits. Voici la liste complete des emojis, leur code et description : +Nous utilisons [gitmoji](https://gitmoji.dev) pour catégoriser nos commits. Voici la liste complète des emojis, leur code et description : <GitmojiList /> <Callout type="info" title="Outils"> - Utilisez le [gitmoji-cli](https://github.com/carloscuesta/gitmoji-cli) ou l'[extension VS Code](https://marketplace.visualstudio.com/items?itemName=seatonjiang.gitmoji-vscode) pour une experience de commit interactive. + Utilisez le [gitmoji-cli](https://github.com/carloscuesta/gitmoji-cli) ou l'[extension VS Code](https://marketplace.visualstudio.com/items?itemName=seatonjiang.gitmoji-vscode) pour une expérience de commit interactive. </Callout>Also applies to: 6-6, 11-11
apps/web/src/components/docs/docs-switcher.tsx-3-3 (1)
3-3: RenameMapimport to avoid shadowing the globalMapconstructor.The static analysis tool correctly identifies that importing
Mapfrom lucide-react shadows the globalMapproperty. This can cause confusion and subtle bugs if the globalMapis needed elsewhere in the file.🔎 Proposed fix
-import { ChevronsUpDown, Sparkles, Globe, Map } from "lucide-react"; +import { ChevronsUpDown, Sparkles, Globe, Map as MapIcon } from "lucide-react";Then update line 19:
- "next-sitemap": Map, + "next-sitemap": MapIcon,Committable suggestion skipped: line range outside the PR's diff.
apps/web/src/lib/mdx-processor.tsx-361-362 (1)
361-362: Word count may be off-by-one for edge cases.
source.split(/\s+/)on an empty string or whitespace-only string returns[""], resulting in a word count of 1 instead of 0. Consider filtering empty strings.Proposed fix
- const words = source.split(/\s+/).length; + const words = source.split(/\s+/).filter(Boolean).length;apps/web/src/lib/mdx-processor.tsx-176-183 (1)
176-183: Internal link handler doesn't spread remaining props.Unlike the external and hash link handlers, the internal
<Link>doesn't spread...props, which may cause loss of attributes likeclassNameoraria-*from the MDX source.Proposed fix
return ( <Link href={href || "#"} className="text-primary hover:underline font-medium" + {...props} > {children} </Link> );apps/web/src/components/mdx/accordion.tsx-131-163 (1)
131-163: Missing cleanup for setTimeout in CopyButton.The
setTimeoutat Line 140 can cause a state update on an unmounted component if the user navigates away within 2 seconds, leading to a React warning.Proposed fix with cleanup
function CopyButton({ id }: { id: string }) { const [, copy] = useCopyToClipboard(); const [checked, setChecked] = useState(false); + const timeoutRef = useRef<NodeJS.Timeout | null>(null); + + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []); const onClick = () => { const url = new URL(window.location.href); url.hash = id; copy(url.toString()); setChecked(true); - setTimeout(() => setChecked(false), 2000); + timeoutRef.current = setTimeout(() => setChecked(false), 2000); };apps/web/src/components/mdx/files.tsx-41-50 (1)
41-50:disabledprop is defined but never used.The
disabledproperty inFolderPropsis declared but not applied to theFoldercomponent logic or the underlyingCollapsible.Proposed fix - either remove or implement
Option 1: Remove unused prop
export interface FolderProps extends HTMLAttributes<HTMLDivElement> { name: string; - disabled?: boolean; /** * Open folder by default * * @defaultValue false */ defaultOpen?: boolean; }Option 2: Implement the disabled behavior
export function Folder({ name, defaultOpen = false, + disabled = false, ...props }: FolderProps): React.ReactElement { const [open, setOpen] = useState(defaultOpen); return ( - <Collapsible open={open} onOpenChange={setOpen} {...props}> + <Collapsible open={open} onOpenChange={disabled ? undefined : setOpen} disabled={disabled} {...props}>apps/web/src/app/[locale]/(docs)/docs/[[...slug]]/page.tsx-130-143 (1)
130-143: UseLinkcomponent for internal navigation.Plain
<a>tags cause full page reloads. For internal routes like/docs/${page.slug}, use theLinkcomponent from@onruntime/translations/nextto enable client-side navigation.Proposed fix
+import { Link } from "@onruntime/translations/next"; // ... at the top with other imports - <a + <Link key={pageIndex} href={`/docs/${page.slug}`} className="group block p-4 rounded-lg border bg-card hover:border-primary transition-colors" > <h3 className="font-medium text-foreground group-hover:text-primary transition-colors mb-1"> {page.title} </h3> {page.description && ( <p className="text-sm text-muted-foreground"> {page.description} </p> )} - </a> + </Link>Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (23)
apps/web/src/content/fr/docs/next-sitemap/getting-started.mdx (1)
20-20: Consider simplifying the heading to "SEO".The term "SEO Optimization" is technically redundant since SEO stands for "Search Engine Optimization." Consider using just "SEO" or "SEO Benefits" for the heading.
apps/web/public/plugins/gitmoji/commands/commit.md (1)
25-31: Add language identifier to the fenced code block.The fenced code block should specify a language identifier for proper syntax highlighting. Consider using
textormarkdownfor the commit structure example.🔎 Proposed fix
-``` +```text <gitmoji> <type> <description> [(#<issue number>)] [optional body]apps/web/src/components/mdx/gitmoji-list.tsx (1)
22-61: Consider adding a fallback UI for empty state.When the fetch fails and returns an empty array, the table will render with headers but no rows. Consider adding a user-friendly message for this scenario.
🔎 Proposed enhancement
export async function GitmojiList() { const gitmojis = await getGitmojis(); + if (gitmojis.length === 0) { + return ( + <div className="p-4 text-center text-muted-foreground"> + Unable to load gitmoji list. Please try again later. + </div> + ); + } + return ( <div className="overflow-x-auto mb-4">apps/web/src/components/mdx/callout.tsx (1)
25-26: Simplify redundant type normalization.The second ternary check
type === "warning" ? "warning" : typeis redundant since "warning" would already be returned by the first condition.🔎 Proposed simplification
- const normalizedType: "info" | "warning" | "error" | "success" = - type === "warn" ? "warning" : type === "warning" ? "warning" : type; + const normalizedType: "info" | "warning" | "error" | "success" = + type === "warn" ? "warning" : type;apps/web/src/locales/fr/layout/navbar.json (1)
7-7: Consider localizing "Docs" to French.The translation uses "Docs" which matches the English value. Consider using "Documentation" for better French localization, unless "Docs" is intentionally kept as a brand term.
apps/web/src/content/en/docs/translations/getting-started.mdx (1)
73-77: Consider adding TypeScript types to example.The example component uses destructured props without type annotations. For documentation promoting type-safety, consider adding types:
function Welcome({ name }: { name: string }) {apps/web/src/app/[locale]/(docs)/layout.tsx (1)
9-11: Add JSDoc comment for exported function.Per coding guidelines, exported functions should have JSDoc comments.
🔎 Proposed fix
+/** + * Layout wrapper for documentation pages. + * Applies docs-specific styles and renders child content. + */ export default function DocsLayout({ children }: DocsLayoutProps) { return <>{children}</>; }apps/web/src/content/en/docs/translations/react.mdx (1)
40-49: Consider adding TypeScript types to component examples.The documentation promotes type-safety but examples lack prop types. Consider adding types for consistency:
function Greeting({ name, itemCount }: { name: string; itemCount: number }) {apps/web/src/content/en/docs/next-sitemap/getting-started.mdx (1)
20-20: Minor: "SEO Optimization" is redundant."SEO" stands for "Search Engine Optimization", so "SEO Optimization" is redundant. Consider using just "SEO" or "Search Engine Optimization".
🔎 Suggested fix
-### SEO Optimization +### SEO Benefitsapps/web/src/components/docs/sidebar.tsx (3)
23-29: Add JSDoc comment for exported function.As per coding guidelines, exported functions should have JSDoc comments.
🔎 Suggested addition
+/** + * Documentation sidebar component with mobile and desktop support. + * Renders navigation sections and pages with active state highlighting. + */ export function DocsSidebar({ metadata, currentProject, className, isMobileOpen = false, setIsMobileOpen, }: DocsSidebarProps) {
41-92: Consider extractingSidebarContentto avoid recreation on each render.
SidebarContentis defined as a nested function component insideDocsSidebar, which means it gets recreated on every render. While this works, it can cause unnecessary re-renders of child components. Consider extracting it or memoizing it.🔎 Suggested refactor
One approach is to pass the needed values as props to an extracted component:
interface SidebarContentProps { metadata: DocsMetadata | null; currentProject: DocsProject; isActivePage: (slug: string) => boolean; setMobileOpen: (open: boolean) => void; } function SidebarContent({ metadata, currentProject, isActivePage, setMobileOpen }: SidebarContentProps) { return ( <div className="flex flex-col h-full"> {/* ... existing content ... */} </div> ); }
64-74: Using array indices as keys.Using
sectionIndexandpageIndexas React keys works for static content, but if sections/pages can be reordered or filtered dynamically, consider using unique identifiers likesection.titleorpage.sluginstead.-{metadata?.sections.map((section, sectionIndex) => ( - <div key={sectionIndex}> +{metadata?.sections.map((section) => ( + <div key={section.title}>-{section.pages.map((page, pageIndex) => ( - <Link - key={pageIndex} +{section.pages.map((page) => ( + <Link + key={page.slug}apps/web/src/components/docs/docs-switcher.tsx (1)
31-63: Add JSDoc comment for the exported function.As per coding guidelines, exported functions should have JSDoc comments describing their purpose and parameters.
🔎 Proposed fix
+/** + * A dropdown component for switching between documentation projects. + * Displays the current project with icon and allows navigation to other project docs. + */ export function DocsSwitcher({ currentProject }: DocsSwitcherProps) {apps/web/src/components/docs/breadcrumb.tsx (1)
16-43: Clean breadcrumb implementation with proper locale-aware navigation.The component correctly differentiates the last breadcrumb item (non-clickable) from prior items (links) and uses the locale-aware
Linkfrom@onruntime/translations/next.Consider adding a JSDoc comment for the exported function per coding guidelines.
apps/web/src/components/docs/docs-config.ts (1)
33-44: Consider deriving valid project IDs from theprojectsarray to avoid drift.The hardcoded checks for
"translations","next-sitemap", and"gitmoji"duplicate theDocsProjecttype. If a new project is added toDocsProject, this function won't recognize it without manual updates.🔎 Proposed fix
+const projectIds = new Set(projects.map((p) => p.id)); + export function getProjectFromSlug(slug: string[] | undefined): DocsProject { if (!slug || slug.length === 0) return "gitmoji"; const firstSegment = slug[0]; - if ( - firstSegment === "translations" || - firstSegment === "next-sitemap" || - firstSegment === "gitmoji" - ) { - return firstSegment; + if (projectIds.has(firstSegment as DocsProject)) { + return firstSegment as DocsProject; } return "gitmoji"; }apps/web/src/components/docs/page-navigation.tsx (1)
33-39: Hardcoded locale for date formatting may not match user preference.The
formatDatefunction uses"en-US"regardless of the user's locale. Consider accepting the locale as a prop or deriving it from context to maintain consistency with the i18n setup.🔎 Proposed fix
Add locale to props and use it:
interface PageNavigationProps { metadata: DocsMetadata | null; currentSlug: string; className?: string; lastModified?: string; + locale?: string; } export function PageNavigation({ metadata, currentSlug, className, lastModified, + locale = "en-US", }: PageNavigationProps) { // ... const formatDate = (date: Date) => { - return new Intl.DateTimeFormat("en-US", { + return new Intl.DateTimeFormat(locale, { month: "2-digit", day: "2-digit", year: "numeric", }).format(date); };apps/web/src/components/mdx/cards.tsx (2)
6-15: Missing JSDoc comments for exported functions.Per coding guidelines, exported functions should have JSDoc comments describing their purpose.
Proposed JSDoc additions
+/** + * Container component that renders children in a responsive 2-column grid. + */ export function Cards(props: HTMLAttributes<HTMLDivElement>) {
25-73: Well-structured Card component with proper link handling.The component correctly handles three render paths (external anchor, internal Link, and div fallback) with appropriate security attributes for external links (
noopener noreferrer). Consider adding JSDoc documentation for consistency with coding guidelines.One minor note: the
propsspread on Line 56 and 62 includes the destructured properties (icon,title,description,href,external) which are already removed via destructuring, so this is safe.apps/web/src/app/[locale]/(docs)/docs/[[...slug]]/page.tsx (1)
31-48: Missing JSDoc for exported functions.Per coding guidelines,
generateStaticParamsandgenerateMetadatashould have JSDoc comments describing their purpose.Also applies to: 50-77
apps/web/src/components/mdx/files.tsx (1)
22-34: Clean implementation of file tree components.The components are well-structured:
Filesprovides a styled containerFilerenders items with customizable iconsFolderuses controlledCollapsiblewith proper state managementConsider adding JSDoc comments for exported functions per coding guidelines.
Also applies to: 52-64, 66-86
apps/web/src/lib/docs.ts (3)
125-177:getDocsPageBySlugis marked async but contains no await operations.The function is wrapped with
cache()and declared async, but performs only synchronous file operations. The async declaration is unnecessary and can be removed.Proposed fix
export const getDocsPageBySlug = cache( - async (slug: string, locale: string = "en"): Promise<DocsPage | null> => { + (slug: string, locale: string = "en"): DocsPage | null => { try { // ... existing synchronous logic } catch (error) { console.error("Error reading docs page:", error); return null; } } );Note: This will also require updating
getDocsPageBySlugs(Line 179-184) to remove the async/await.
49-72: Solid implementation of docs page discovery.The caching strategy using React's
cache()is appropriate for server components. Error handling with try/catch and graceful fallbacks (empty arrays) ensures resilience.The recursive
getAllMdxFilesfunction correctly handles:
- Nested directory structures
index.mdxfiles for directory slugs- Frontmatter extraction via gray-matter
- Last modified timestamps
Consider adding JSDoc comments for the exported functions per coding guidelines.
Also applies to: 74-123
238-275: Breadcrumb generation has O(n×m) complexity.
getDocsBreadcrumbscallsgetAllDocsPages(locale)for each slug segment (Line 263). While this is mitigated by React'scache(), the linear search on each iteration could be optimized by building a lookup map once.Optimization suggestion (optional)
export const getDocsBreadcrumbs = cache( (slugs: string[], locale: string = "en"): { title: string; href: string }[] => { const breadcrumbs: { title: string; href: string }[] = [ { title: "Docs", href: "/docs" }, ]; if (slugs.length === 0) return breadcrumbs; + // Build lookup map once + const allPages = getAllDocsPages(locale); + const pageMap = new Map(allPages.map(p => [p.slugs.join("/"), p])); + let currentPath = "/docs"; for (let i = 0; i < slugs.length; i++) { // ... project handling unchanged const currentSlugs = slugs.slice(0, i + 1); - const page = getAllDocsPages(locale).find( - (p) => p.slugs.join("/") === currentSlugs.join("/") - ); + const page = pageMap.get(currentSlugs.join("/")); // ... }
| export function ConditionalLayout({ children, hideOnDocs = false }: ConditionalLayoutProps) { | ||
| const pathname = usePathname(); | ||
| const isDocsPage = pathname?.includes("/docs"); | ||
|
|
||
| if (hideOnDocs && isDocsPage) { | ||
| return null; | ||
| } | ||
|
|
||
| return <>{children}</>; | ||
| } |
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.
Improve pathname matching precision and add JSDoc.
The current implementation has two issues:
-
Missing JSDoc comment - The exported function needs documentation per coding guidelines.
-
Pathname matching is too broad - Line 13 uses
pathname?.includes("/docs")which would incorrectly match paths like/mydocs,/api/docs-endpoint, or/documentation. This could hide the navbar/footer on unintended pages.
🔎 Proposed fix
+/**
+ * Conditionally renders children based on the current pathname.
+ * Can be configured to hide children on documentation pages.
+ *
+ * @param children - The child elements to render
+ * @param hideOnDocs - Whether to hide children when on docs pages (default: false)
+ * @returns The children or null based on the condition
+ */
export function ConditionalLayout({ children, hideOnDocs = false }: ConditionalLayoutProps) {
const pathname = usePathname();
- const isDocsPage = pathname?.includes("/docs");
+ const isDocsPage = pathname?.match(/^\/[^/]+\/docs(\/|$)/);
if (hideOnDocs && isDocsPage) {
return null;
}
return <>{children}</>;
}The regex pattern /^\/[^/]+\/docs(\/|$)/ matches paths that have /docs as a complete segment after the locale (e.g., /en/docs, /fr/docs/guide), but not paths where "docs" appears within another segment.
As per coding guidelines: "Always add JSDoc comments for exported functions and classes"
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/components/layout/conditional-layout.tsx around lines 11 to 20,
add a JSDoc comment for the exported ConditionalLayout function and tighten the
docs-page detection: replace the broad pathname?.includes("/docs") check with a
regex that matches "/docs" as a full path segment after the locale (for example
use /^\/[^/]+\/docs(\/|$)/ or equivalent) so paths like "/mydocs" or
"/api/docs-endpoint" no longer match; keep the existing hideOnDocs conditional
and return null when the regex test passes.
| export const Callout = forwardRef<HTMLDivElement, CalloutProps>( | ||
| ({ className, children, title, type = "info", icon, ...props }, ref) => { | ||
| const normalizedType: "info" | "warning" | "error" | "success" = | ||
| type === "warn" ? "warning" : type === "warning" ? "warning" : type; | ||
|
|
||
| return ( | ||
| <div | ||
| ref={ref} | ||
| className={cn( | ||
| "flex gap-2 my-4 rounded-xl border bg-card p-3 ps-1 text-sm text-card-foreground shadow-md", | ||
| className, | ||
| )} | ||
| {...props} | ||
| style={ | ||
| { | ||
| "--callout-color": | ||
| normalizedType === "info" | ||
| ? "#3b82f6" | ||
| : normalizedType === "warning" | ||
| ? "#f59e0b" | ||
| : normalizedType === "error" | ||
| ? "#ef4444" | ||
| : normalizedType === "success" | ||
| ? "#10b981" | ||
| : "#6b7280", | ||
| ...props.style, | ||
| } as object | ||
| } | ||
| > | ||
| <div | ||
| role="none" | ||
| className="w-0.5 rounded-sm" | ||
| style={{ | ||
| backgroundColor: "rgb(from var(--callout-color) r g b / 0.5)", | ||
| }} | ||
| /> | ||
| {icon ?? | ||
| { | ||
| info: ( | ||
| <Info | ||
| className={iconClass} | ||
| style={{ fill: "var(--callout-color)" }} | ||
| /> | ||
| ), | ||
| warning: ( | ||
| <TriangleAlert | ||
| className={iconClass} | ||
| style={{ fill: "var(--callout-color)" }} | ||
| /> | ||
| ), | ||
| error: ( | ||
| <CircleX | ||
| className={iconClass} | ||
| style={{ fill: "var(--callout-color)" }} | ||
| /> | ||
| ), | ||
| success: ( | ||
| <CircleCheck | ||
| className={iconClass} | ||
| style={{ fill: "var(--callout-color)" }} | ||
| /> | ||
| ), | ||
| }[normalizedType]} | ||
| <div className="flex flex-col gap-2 min-w-0 flex-1"> | ||
| {title && <p className="font-medium !my-0 text-foreground">{title}</p>} | ||
| <div | ||
| className="text-muted-foreground empty:hidden [&>:first-child]:mt-0 [&>:last-child]:mb-0 [&>*]:!text-sm" | ||
| > | ||
| {children} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }, | ||
| ); |
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 | 🟠 Major
Add JSDoc documentation for exported component.
As per coding guidelines, exported functions and classes should have JSDoc comments. Consider adding documentation describing the component's purpose, props, and usage examples.
🔎 Example JSDoc
+/**
+ * A styled callout component for highlighting important information in documentation.
+ * Supports multiple types (info, warning, error, success) with corresponding icons and colors.
+ *
+ * @example
+ * <Callout type="info" title="Note">
+ * This is an informational callout.
+ * </Callout>
+ */
export const Callout = forwardRef<HTMLDivElement, CalloutProps>(Based on coding guidelines requiring JSDoc for exported functions and classes.
📝 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 const Callout = forwardRef<HTMLDivElement, CalloutProps>( | |
| ({ className, children, title, type = "info", icon, ...props }, ref) => { | |
| const normalizedType: "info" | "warning" | "error" | "success" = | |
| type === "warn" ? "warning" : type === "warning" ? "warning" : type; | |
| return ( | |
| <div | |
| ref={ref} | |
| className={cn( | |
| "flex gap-2 my-4 rounded-xl border bg-card p-3 ps-1 text-sm text-card-foreground shadow-md", | |
| className, | |
| )} | |
| {...props} | |
| style={ | |
| { | |
| "--callout-color": | |
| normalizedType === "info" | |
| ? "#3b82f6" | |
| : normalizedType === "warning" | |
| ? "#f59e0b" | |
| : normalizedType === "error" | |
| ? "#ef4444" | |
| : normalizedType === "success" | |
| ? "#10b981" | |
| : "#6b7280", | |
| ...props.style, | |
| } as object | |
| } | |
| > | |
| <div | |
| role="none" | |
| className="w-0.5 rounded-sm" | |
| style={{ | |
| backgroundColor: "rgb(from var(--callout-color) r g b / 0.5)", | |
| }} | |
| /> | |
| {icon ?? | |
| { | |
| info: ( | |
| <Info | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| warning: ( | |
| <TriangleAlert | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| error: ( | |
| <CircleX | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| success: ( | |
| <CircleCheck | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| }[normalizedType]} | |
| <div className="flex flex-col gap-2 min-w-0 flex-1"> | |
| {title && <p className="font-medium !my-0 text-foreground">{title}</p>} | |
| <div | |
| className="text-muted-foreground empty:hidden [&>:first-child]:mt-0 [&>:last-child]:mb-0 [&>*]:!text-sm" | |
| > | |
| {children} | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| }, | |
| ); | |
| /** | |
| * A styled callout component for highlighting important information in documentation. | |
| * Supports multiple types (info, warning, error, success) with corresponding icons and colors. | |
| * | |
| * @example | |
| * <Callout type="info" title="Note"> | |
| * This is an informational callout. | |
| * </Callout> | |
| */ | |
| export const Callout = forwardRef<HTMLDivElement, CalloutProps>( | |
| ({ className, children, title, type = "info", icon, ...props }, ref) => { | |
| const normalizedType: "info" | "warning" | "error" | "success" = | |
| type === "warn" ? "warning" : type === "warning" ? "warning" : type; | |
| return ( | |
| <div | |
| ref={ref} | |
| className={cn( | |
| "flex gap-2 my-4 rounded-xl border bg-card p-3 ps-1 text-sm text-card-foreground shadow-md", | |
| className, | |
| )} | |
| {...props} | |
| style={ | |
| { | |
| "--callout-color": | |
| normalizedType === "info" | |
| ? "#3b82f6" | |
| : normalizedType === "warning" | |
| ? "#f59e0b" | |
| : normalizedType === "error" | |
| ? "#ef4444" | |
| : normalizedType === "success" | |
| ? "#10b981" | |
| : "#6b7280", | |
| ...props.style, | |
| } as object | |
| } | |
| > | |
| <div | |
| role="none" | |
| className="w-0.5 rounded-sm" | |
| style={{ | |
| backgroundColor: "rgb(from var(--callout-color) r g b / 0.5)", | |
| }} | |
| /> | |
| {icon ?? | |
| { | |
| info: ( | |
| <Info | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| warning: ( | |
| <TriangleAlert | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| error: ( | |
| <CircleX | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| success: ( | |
| <CircleCheck | |
| className={iconClass} | |
| style={{ fill: "var(--callout-color)" }} | |
| /> | |
| ), | |
| }[normalizedType]} | |
| <div className="flex flex-col gap-2 min-w-0 flex-1"> | |
| {title && <p className="font-medium !my-0 text-foreground">{title}</p>} | |
| <div | |
| className="text-muted-foreground empty:hidden [&>:first-child]:mt-0 [&>:last-child]:mb-0 [&>*]:!text-sm" | |
| > | |
| {children} | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In apps/web/src/components/mdx/callout.tsx around lines 23 to 97, add a JSDoc
block immediately above the exported Callout declaration that documents the
component purpose, its props (className, children, title, type, icon and any
others passed via ...props) including allowed values for type ("info" |
"warning" | "error" | "success"), the ref behavior, and a short usage example;
include @param tags for each prop, an @returns tag stating it renders a styled
callout div, and an @example showing minimal JSX usage so the exported component
now meets the codebase JSDoc guideline.
| const getGitmojis = unstable_cache( | ||
| async (): Promise<Gitmoji[]> => { | ||
| const response = await fetch( | ||
| "https://raw.githubusercontent.com/carloscuesta/gitmoji/master/packages/gitmojis/src/gitmojis.json" | ||
| ); | ||
| const data = await response.json(); | ||
| return data.gitmojis; | ||
| }, | ||
| ["gitmojis"], | ||
| { revalidate: 86400 } // Cache for 24 hours | ||
| ); |
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 error handling for the fetch operation.
The async function lacks try-catch error handling, violating the coding guideline. If the GitHub API is unavailable or returns invalid JSON, the page will crash instead of degrading gracefully.
As per coding guidelines, all async functions must handle errors with try/catch blocks.
🔎 Proposed fix with error handling
const getGitmojis = unstable_cache(
async (): Promise<Gitmoji[]> => {
+ try {
const response = await fetch(
"https://raw.githubusercontent.com/carloscuesta/gitmoji/master/packages/gitmojis/src/gitmojis.json"
);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch gitmojis: ${response.status}`);
+ }
const data = await response.json();
return data.gitmojis;
+ } catch (error) {
+ console.error("Failed to load gitmojis:", error);
+ return [];
+ }
},
["gitmojis"],
{ revalidate: 86400 } // Cache for 24 hours
);🤖 Prompt for AI Agents
In apps/web/src/components/mdx/gitmoji-list.tsx around lines 10 to 20, the async
getGitmojis function lacks error handling for network failures and invalid JSON;
wrap the fetch/response.json logic in a try/catch, check response.ok before
parsing and handle non-OK responses, log the error (or use the app logger)
inside the catch, and return a safe fallback (e.g., an empty array) so the page
degrades gracefully instead of crashing.
| ```typescript | ||
| // types/translations.d.ts | ||
| import "onruntime/translations"; | ||
|
|
||
| declare module "@onruntime/translations" { | ||
| interface TranslationNamespaces { | ||
| common: typeof import("../locales/en/common.json"); | ||
| auth: typeof import("../locales/en/auth.json"); | ||
| } | ||
| } | ||
| ``` |
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.
Fix the incorrect import path.
Line 59 uses "onruntime/translations" but should be "@onruntime/translations" to match the package name used throughout the rest of the documentation (lines 12, 75, 99).
🔎 Proposed fix
// types/translations.d.ts
-import "onruntime/translations";
+import "@onruntime/translations";
declare module "@onruntime/translations" {
interface TranslationNamespaces {🤖 Prompt for AI Agents
In apps/web/src/content/en/docs/translations/installation.mdx around lines 57 to
67, the import on line 59 uses "onruntime/translations" but should be
"@onruntime/translations"; update that import string to
"@onruntime/translations" so it matches the package name used elsewhere in the
docs and keeps the example correct.
| { | ||
| "title": "@onruntime/next-sitemap", | ||
| "description": "Sitemap generator for Next.js", | ||
| "sections": [ | ||
| { | ||
| "title": "Getting Started", | ||
| "pages": [ | ||
| { | ||
| "slug": "getting-started", | ||
| "title": "Getting Started", | ||
| "description": "What is @onruntime/next-sitemap?" | ||
| }, | ||
| { | ||
| "slug": "installation", | ||
| "title": "Installation", | ||
| "description": "Install and set up the package" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "title": "Configuration", | ||
| "pages": [ | ||
| { | ||
| "slug": "configuration", | ||
| "title": "Configuration", | ||
| "description": "Configure sitemap generation" | ||
| }, | ||
| { | ||
| "slug": "api", | ||
| "title": "API Reference", | ||
| "description": "Complete API documentation" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
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.
Fix locale inconsistency: English content in French directory.
This file is located in apps/web/src/content/fr/docs/ but contains English content. All titles, descriptions, and section names are in English, which is inconsistent with the French locale path.
Either:
- Translate all content to French to match the
/fr/locale path, or - Move this file to the corresponding English locale path
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Test command failed. The step 'pnpm run test' exited with code 1.
🤖 Prompt for AI Agents
apps/web/src/content/fr/docs/next-sitemap/meta.json lines 1-36: The file lives
in the French docs folder but all titles, descriptions and section names are in
English, causing a locale mismatch; fix by either translating every title,
description and section name into French to match the /fr/ locale (e.g.,
"Getting Started" → "Commencer", "Installation" → "Installation",
"Configuration" → "Configuration", "API Reference" → "Référence de l'API", and
translate the descriptions) or move this file to the English locale folder
(e.g., apps/web/src/content/en/docs/) so content and path match; apply one of
these two options consistently and update any references or routing metadata
accordingly.
| { | ||
| "title": "@onruntime/translations", | ||
| "description": "Internationalization for React & Next.js", | ||
| "sections": [ | ||
| { | ||
| "title": "Getting Started", | ||
| "pages": [ | ||
| { | ||
| "slug": "getting-started", | ||
| "title": "Getting Started", | ||
| "description": "What is @onruntime/translations?" | ||
| }, | ||
| { | ||
| "slug": "installation", | ||
| "title": "Installation", | ||
| "description": "Install and set up the package" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "title": "Usage", | ||
| "pages": [ | ||
| { | ||
| "slug": "react", | ||
| "title": "React Usage", | ||
| "description": "Using translations in React apps" | ||
| }, | ||
| { | ||
| "slug": "nextjs", | ||
| "title": "Next.js Usage", | ||
| "description": "Using translations in Next.js apps" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "title": "API Reference", | ||
| "pages": [ | ||
| { | ||
| "slug": "api", | ||
| "title": "API Reference", | ||
| "description": "Complete API documentation" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
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.
Fix locale inconsistency: English content in French directory.
This metadata file is located in apps/web/src/content/fr/docs/ but contains entirely English content. All titles, descriptions, and section names are in English, which is inconsistent with the French locale path.
Either:
- Translate all content to French to match the
/fr/locale path, or - Move this file to the corresponding English locale path
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Test command failed. The step 'pnpm run test' exited with code 1.
| export async function processMDX( | ||
| source: string, | ||
| locale: string = "en", | ||
| ): Promise<ProcessedMDX> { | ||
| const headings: HeadingData[] = []; | ||
|
|
||
| const { content } = await compileMDX({ | ||
| source, | ||
| components: mdxComponents, | ||
| options: { | ||
| mdxOptions: { | ||
| remarkPlugins: [remarkGfm, remarkExtractHeadings(headings)], | ||
| rehypePlugins: [ | ||
| rehypeSlug, | ||
| [ | ||
| rehypeAutolinkHeadings, | ||
| { | ||
| behavior: "wrap", | ||
| properties: { | ||
| className: ["anchor"], | ||
| }, | ||
| }, | ||
| ], | ||
| ], | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| const words = source.split(/\s+/).length; | ||
| const minutes = Math.ceil(words / 200); | ||
| const minReadText = locale === "fr" ? "min de lecture" : "min read"; | ||
| const readingTime: ReadingTime = { | ||
| text: `${minutes} ${minReadText}`, | ||
| minutes, | ||
| words, | ||
| }; | ||
|
|
||
| return { | ||
| content, | ||
| headings, | ||
| readingTime, | ||
| }; | ||
| } |
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 error handling for async MDX compilation.
Per coding guidelines, all async functions must handle errors with try/catch blocks. If compileMDX throws (e.g., invalid MDX syntax), the error will propagate unhandled.
Proposed fix with error handling
+/**
+ * Processes MDX source content and returns compiled React content,
+ * extracted headings, and reading time metadata.
+ */
export async function processMDX(
source: string,
locale: string = "en",
): Promise<ProcessedMDX> {
+ try {
const headings: HeadingData[] = [];
const { content } = await compileMDX({
source,
components: mdxComponents,
options: {
mdxOptions: {
remarkPlugins: [remarkGfm, remarkExtractHeadings(headings)],
rehypePlugins: [
rehypeSlug,
[
rehypeAutolinkHeadings,
{
behavior: "wrap",
properties: {
className: ["anchor"],
},
},
],
],
},
},
});
const words = source.split(/\s+/).length;
const minutes = Math.ceil(words / 200);
const minReadText = locale === "fr" ? "min de lecture" : "min read";
const readingTime: ReadingTime = {
text: `${minutes} ${minReadText}`,
minutes,
words,
};
return {
content,
headings,
readingTime,
};
+ } catch (error) {
+ console.error("Error processing MDX content:", error);
+ throw error;
+ }
}🤖 Prompt for AI Agents
In apps/web/src/lib/mdx-processor.tsx around lines 333 to 375, the async call to
compileMDX is not wrapped in a try/catch so compilation errors will propagate
unhandled; wrap the compileMDX call (and any subsequent processing dependent on
it) in a try/catch, log or annotate the caught error with context
(file/locale/operation) and either rethrow a wrapped error or return a safe
fallback (e.g., empty content and readingTime) according to project
error-handling conventions so callers receive a consistent response instead of
an unhandled rejection.
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.
Pull request overview
This PR introduces a comprehensive documentation system for the project, including guides for Gitmoji commit conventions, the @onruntime/translations package, and the @onruntime/next-sitemap package. Documentation is provided in multiple languages (Swedish, Portuguese, Polish, Dutch, Korean, Japanese, Italian, and Hindi) with consistent structure and interactive components.
Key changes:
- Added multilingual documentation for Gitmoji conventions, translations library, and sitemap generator
- Implemented consistent documentation structure across all supported languages
- Included interactive components (GitmojiList, Callout, Files, Cards) for enhanced user experience
Reviewed changes
Copilot reviewed 176 out of 245 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/content/sv/docs/gitmoji/emojis.mdx | Swedish Gitmoji emoji reference documentation |
| apps/web/src/content/pt/docs/translations/*.mdx | Portuguese translation library documentation (React, Next.js, installation, getting started, API) |
| apps/web/src/content/pt/docs/next-sitemap/*.mdx | Portuguese sitemap generator documentation (installation, getting started, configuration, API) |
| apps/web/src/content/pt/docs/gitmoji/*.mdx | Portuguese Gitmoji documentation (with AI, getting started, emojis) |
| apps/web/src/content/pl/docs/translations/*.mdx | Polish translation library documentation |
| apps/web/src/content/pl/docs/next-sitemap/*.mdx | Polish sitemap generator documentation |
| apps/web/src/content/pl/docs/gitmoji/*.mdx | Polish Gitmoji documentation |
| apps/web/src/content/nl/docs/translations/*.mdx | Dutch translation library documentation |
| apps/web/src/content/nl/docs/next-sitemap/*.mdx | Dutch sitemap generator documentation |
| apps/web/src/content/nl/docs/gitmoji/*.mdx | Dutch Gitmoji documentation |
| apps/web/src/content/ko/docs/translations/*.mdx | Korean translation library documentation |
| apps/web/src/content/ko/docs/next-sitemap/*.mdx | Korean sitemap generator documentation |
| apps/web/src/content/ko/docs/gitmoji/*.mdx | Korean Gitmoji documentation |
| apps/web/src/content/ja/docs/translations/*.mdx | Japanese translation library documentation |
| apps/web/src/content/ja/docs/next-sitemap/*.mdx | Japanese sitemap generator documentation |
| apps/web/src/content/ja/docs/gitmoji/*.mdx | Japanese Gitmoji documentation |
| apps/web/src/content/it/docs/translations/*.mdx | Italian translation library documentation |
| apps/web/src/content/it/docs/next-sitemap/*.mdx | Italian sitemap generator documentation |
| apps/web/src/content/it/docs/gitmoji/*.mdx | Italian Gitmoji documentation |
| apps/web/src/content/hi/docs/translations/*.mdx | Hindi translation library documentation (React, Next.js) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.