Enhance documentation and SEO for brand assets#543
Conversation
- Updated AGENTS.md to include key rules and a directory map for better navigation and understanding of the project structure. - Added SEO.md to document specific SEO considerations for the editor's public site, including image indexing and metadata requirements. - Modified BrandPage component to use unoptimized images for better SEO performance, ensuring direct URLs are available for Google Image indexing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughExpanded documentation with detailed guidance on editor architecture, SEO best practices, and Next.js Image optimization. Added unoptimized prop to Image components in BrandPage to align with direct PNG URL indexing requirements documented in new SEO guidelines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
editor/app/(www)/(brand)/brand/page.tsx (1)
253-269:titleandlabelprops are always passed identical values.Every call site passes the same string to both
titleandlabel(e.g.,"Light"/"Light").titleis only used inside the download control's<span>, whilelabelappears in the card header — they serve the same purpose here. Consider collapsing into a single prop to reduce surface area, or differentiate their usage if future divergence is planned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor/app/`(www)/(brand)/brand/page.tsx around lines 253 - 269, The component BrandAssetCard duplicates title and label; consolidate to a single prop (e.g., label) by removing title from the props list and replacing any internal uses of title (such as the download control's <span>) with label, update the function signature for BrandAssetCard to accept label only (keeping variant, previewSrc, pngDownloadSrc, svgDownloadSrc, alt), and update every call site to stop passing title and pass only label (or rename the passed argument accordingly); if you prefer to keep both for future divergence instead, explicitly document and use distinct names where they differ, but do not leave identical props in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@editor/AGENTS.md`:
- Line 12: Update the note about Edge entrypoints to clarify runtimes: state
that proxy.ts replaces middleware.ts for Node.js runtime in Next.js 16 (same
semantics but export renamed to proxy), while middleware.ts is still used for
Edge runtime scenarios (though deprecated) so developers should choose proxy.ts
for Node.js runtime and middleware.ts only for Edge-specific needs; reference
the files proxy.ts and middleware.ts and mention Next.js 16 and Edge vs Node.js
runtime distinctions in the single sentence.
---
Nitpick comments:
In `@editor/app/`(www)/(brand)/brand/page.tsx:
- Around line 253-269: The component BrandAssetCard duplicates title and label;
consolidate to a single prop (e.g., label) by removing title from the props list
and replacing any internal uses of title (such as the download control's <span>)
with label, update the function signature for BrandAssetCard to accept label
only (keeping variant, previewSrc, pngDownloadSrc, svgDownloadSrc, alt), and
update every call site to stop passing title and pass only label (or rename the
passed argument accordingly); if you prefer to keep both for future divergence
instead, explicitly document and use distinct names where they differ, but do
not leave identical props in place.
| - **Auth is special**: `app/(auth)` is security-critical. **Do not modify** routes/flows there. | ||
| - **Public API is versioned**: treat `app/(api)/(public)/v1` as **backwards-compatible** (additive changes only unless you’re intentionally breaking/v2-ing). | ||
| - **Layouts are per route group**: there isn’t a single shared root layout across the whole `app/` tree — top-level route groups own their root `layout.tsx`/metadata. | ||
| - **Edge entrypoint is `proxy.ts`**: on Next.js 16 this replaces `middleware.ts` (same runtime + semantics). Don’t add a new `middleware.ts`. |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 16 proxy.ts middleware replacement
💡 Result:
In Next.js 16, proxy.ts replaces middleware.ts for request interception on the Node.js runtime (to make the “network boundary” explicit). To migrate, rename middleware.ts → proxy.ts and rename your exported function to proxy—the logic can stay the same. [1][2]
Migration (recommended):
- Run the codemod:
npx@next/codemod@canary middleware-to-proxy .[2] - Or manually:
// proxy.ts
import type { NextRequest } from "next/server";
import { NextResponse } from "next/server";
export function proxy(request: NextRequest) {
// your existing redirects/rewrites/header logic
return NextResponse.next();
}Note: middleware.ts still exists for Edge runtime use cases, but it’s deprecated and planned for removal in a future version. [1]
Sources:
[1] Next.js 16 blog (proxy.ts formerly middleware.ts)
[2] Next.js docs message: Renaming Middleware to Proxy
Clarify the runtime distinction for proxy.ts vs middleware.ts in Next.js 16.
The guidance on line 12 is mostly correct but incomplete. proxy.ts replaces middleware.ts for Node.js runtime in Next.js 16 with equivalent semantics (function logic remains the same—just rename the export to proxy). However, middleware.ts still exists for Edge runtime use cases (though deprecated). Expand the note to clarify this distinction so developers understand when to use each approach.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor/AGENTS.md` at line 12, Update the note about Edge entrypoints to
clarify runtimes: state that proxy.ts replaces middleware.ts for Node.js runtime
in Next.js 16 (same semantics but export renamed to proxy), while middleware.ts
is still used for Edge runtime scenarios (though deprecated) so developers
should choose proxy.ts for Node.js runtime and middleware.ts only for
Edge-specific needs; reference the files proxy.ts and middleware.ts and mention
Next.js 16 and Edge vs Node.js runtime distinctions in the single sentence.
Summary by CodeRabbit
Documentation
Bug Fixes