Skip to content

feat: create sitemap#1666

Open
edulennert wants to merge 2 commits intodevfrom
feat/seo-improvements
Open

feat: create sitemap#1666
edulennert wants to merge 2 commits intodevfrom
feat/seo-improvements

Conversation

@edulennert
Copy link
Contributor

@edulennert edulennert commented Feb 27, 2026

@vercel
Copy link

vercel bot commented Feb 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
anticapture Ready Ready Preview, Comment Feb 27, 2026 5:04pm
anticapture-storybook Ready Ready Preview, Comment Feb 27, 2026 5:04pm

Request Review

@pikonha
Copy link
Member

pikonha commented Mar 4, 2026

@claude

@claude
Copy link

claude bot commented Mar 4, 2026

Claude finished @pikonha's task in 59s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze and provide review

Overall the implementation is clean and well-structured. Here are a few observations:

Issues / Suggestions

  1. VERCEL_URL is the deployment URL, not the production URL (apps/dashboard/app/sitemap.ts:8-9)

    VERCEL_URL resolves to the unique deployment URL (e.g., anticapture-git-feat-xxx.vercel.app), which changes per deployment. For the production URL, use VERCEL_PROJECT_PRODUCTION_URL instead. This matters because search engines will index the production URL and the sitemap should consistently point there.

    // Current
    process.env.VERCEL_URL ? `https://${process.env.VERCEL_URL}` : ...
    
    // Suggested
    process.env.VERCEL_PROJECT_PRODUCTION_URL
      ? `https://${process.env.VERCEL_PROJECT_PRODUCTION_URL}`
      : ...

    Fix this →

  2. panel page is missing from the sitemap (apps/dashboard/app/sitemap.ts:24-61)

    PAGES_CONSTANTS.panel.page ("panel") is defined but not included in staticPages. Is this intentional (e.g., requires authentication)? If it's public-facing it should be included for SEO.

  3. Fragile "/" check for DAO overview (apps/dashboard/app/sitemap.ts:67-69)

    The check subPage === "/" relies on the internal string value of PAGES_CONSTANTS.daoOverview.page. If that value changes, URL construction silently breaks. Consider using a constant or comparing against the constant itself:

    subPage === PAGES_CONSTANTS.daoOverview.page

    Fix this →

  4. lastModified: new Date() reflects generation time, not content change time

    This is a common tradeoff and acceptable here since the pages don't have trackable last-edit timestamps. No action needed, just worth being aware that search engines may re-crawl more frequently than necessary.

What looks good

  • Correct use of MetadataRoute.Sitemap type from Next.js
  • Sensible priority values (homepage at 1.0, DAO overview pages at 0.9, sub-pages at 0.8)
  • Using ALL_DAOS to enumerate DAO pages dynamically keeps the sitemap in sync with new DAOs
  • The as const assertion on changeFrequency in daoSubPages.map avoids type widening to string
  • Clean separation between staticPages and daoPages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants