Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a client-side GeneratePage component, a server route for dynamic Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant GenerateClient as GeneratePageClient
participant ServerPage as [repo] Server Component
participant API as /api/generate
participant Markdown as MarkdownPreview
Browser->>ServerPage: Request /generate/[repo]
ServerPage-->>Browser: Render page + metadata (pre-filled repoSlug)
Browser->>GenerateClient: Mount with repoSlug prop
GenerateClient->>GenerateClient: set initial input from repoSlug
Browser->>API: POST /api/generate { url }
API-->>GenerateClient: 200 { markdown }
GenerateClient->>Markdown: render markdown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/app/generate/`[repo]/page.tsx:
- Around line 4-8: The PageProps/route currently assumes a single path segment
for repo but must be a catch-all to accept owner/repo; rename the route folder
from [repo] to [...repo], change the params type in PageProps to repo: string[]
and in your page/component (and in GeneratePageServer) join params.repo with '/'
to produce the full "owner/repo" string before use; update any
destructuring/usages that expect a string to instead join the array first.
- Around line 4-8: The PageProps interface currently types params as a plain
object but Next.js 15+ provides params as a Promise; update the PageProps
declaration so params is Promise<{ repo: string }>, and also update any related
types for generateMetadata and the page component (e.g., generateMetadata,
default export Page or page function) to accept/await Promise<{ repo: string }>,
ensuring the places where you already await params (around the existing awaits
on lines 14 and 48) match the type signature.
In `@src/app/generate/GeneratePageClient.tsx`:
- Around line 72-77: The SearchInput component is not accepting or forwarding
aria attributes because SearchInputProps only declares onGenerate, isLoading and
initialValue; add an optional ariaLabel (or a generic props spread) to
SearchInputProps and accept it in the SearchInput component, then pass that
value to the underlying input element (e.g., use a prop name like ariaLabel and
apply it as aria-label on the <input> inside SearchInput) so the aria attribute
from GeneratePageClient's SearchInput usage is actually applied.
In `@src/app/layout.tsx`:
- Around line 30-45: Replace hardcoded site URLs so they follow metadataBase: in
src/app/layout.tsx remove or set openGraph.url to "/" (Next resolves it against
metadataBase) instead of "https://readmegen-ai.vercel.app", and update the
JSON-LD url value to use the same env-driven base (metadataBase or
process.env.NEXT_PUBLIC_SITE_URL) + path rather than the hardcoded domain;
ensure both openGraph.url and the JSON-LD `url` reference the single source
(metadataBase / NEXT_PUBLIC_SITE_URL) so custom domains pick up automatically.
🧹 Nitpick comments (4)
src/app/generate/GeneratePageClient.tsx (2)
17-25:document.titleoverride conflicts with Next.jsgenerateMetadata.The
[repo]/page.tsxserver component already exportsgenerateMetadatathat sets the page title via Next.js's Metadata API. ThisuseEffectwill overwrite it on the client after hydration, potentially causing a visible title flicker and duplicating title logic in two places.Consider removing this
useEffectand relying solely on the server-side metadata for SEO titles. If you still need it as a fallback for the non-slug/generateroute, limit it to only that case.Suggested change
- // Optional: Update document title for SPA navigation - useEffect(() => { - if (repoSlug) { - const repoName = repoSlug.split("/").pop(); - document.title = `Generate README for ${repoName} | ReadmeGenAI`; - } else { - document.title = "ReadmeGenAI – AI GitHub README Generator"; - } - }, [repoSlug]);
27-65: Error UX:alert()is jarring; consider inline error display instead.Using
alert()(Line 62) blocks the UI thread and is a poor UX pattern for a polished product. TheSearchInputcomponent already has inline error display capability. Consider surfacing generation errors through state that renders inline, similar to how validation errors are shown.src/app/generate/[repo]/page.tsx (1)
14-24: User-controlledrepoparam is interpolated into metadata without sanitization.The
repoparameter comes directly from the URL and is interpolated intotitle,description, and Open Graph fields. While Next.js's Metadata API should escape these values when rendering HTML<meta>tags, consider adding basic validation (e.g., checking the format matchesowner/repowith allowed characters) to avoid garbage or malicious values in your metadata.Also applies to: 46-49
src/app/layout.tsx (1)
67-84: Remove manual<head>wrapper and update JSON-LD configuration per Next.js App Router best practices.In Next.js App Router, place JSON-LD
<script>elements directly in the component body (not wrapped in a manual<head>tag), as Next.js automatically manages the head. Also, updateapplicationCategoryto"DeveloperApplication"(Google's rich results guidance recognizes this value, not"DeveloperTool"), and use the environment variable for consistency with the metadata object already defined in this file.Suggested refactor
return ( <html lang="en"> - <head> - {/* JSON-LD structured data */} - <script - type="application/ld+json" - dangerouslySetInnerHTML={{ - __html: JSON.stringify({ - "@context": "https://schema.org", - "@type": "SoftwareApplication", - name: "ReadmeGenAI", - applicationCategory: "DeveloperTool", - operatingSystem: "Web", - description: - "AI-powered GitHub README generator that creates markdown documentation automatically.", - url: "https://readmegen-ai.vercel.app", - }), - }} - /> - </head> <body className={`${geistSans.variable} ${geistMono.variable} antialiased`} > + <script + type="application/ld+json" + dangerouslySetInnerHTML={{ + __html: JSON.stringify({ + "@context": "https://schema.org", + "@type": "SoftwareApplication", + name: "ReadmeGenAI", + applicationCategory: "DeveloperApplication", + operatingSystem: "Web", + description: + "AI-powered GitHub README generator that creates markdown documentation automatically.", + url: process.env.NEXT_PUBLIC_SITE_URL || "https://readmegen-ai.vercel.app", + }), + }} + /> {children} </body>
|
@copilot In In |
Co-authored-by: naheel0 <191262736+naheel0@users.noreply.github.com>
Fix Next.js 15+ params handling and environment-driven metadata URLs
🚀 BΞYTΞFLʘW | Pull Request Protocol
PR Type: (Choose one:
feat|fix|refactor|docs|perf)Issue Link: Fixes #
📝 System Summary
Provide a concise brief of the changes introduced to the stream.
🛠️ Technical Changes
.........🧪 Quality Assurance (QA)
npm run buildexecuted without errors.🖼️ Visual Evidence
If this PR affects the UI, drop a screenshot or GIF below:
📡 Developer Authorization
Authorized by: @naheel0
Timestamp: {{ 15/2/2026}}