-
Notifications
You must be signed in to change notification settings - Fork 0
feat: integrate sanity cms with admin interface (web-370) #5
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
Conversation
- add .cursorrules with coding standards and guidelines - configure eslint exception for env.d.ts - enforce english-only code policy
- add sanity cms integration with studio at /admin route - create centralized config in src/lib/sanity.config.ts - add sanity client and api queries - create blogPost schema matching markdown structure - implement environment variable management - add sanity dependencies and types
- create getAllPosts utility to combine markdown and sanity posts - update all pages to use unified post collection - add sanity post detection and rendering in PostDetails - convert portable text to html for sanity posts - maintain consistent post structure across sources
- add detailed step-by-step guide for creating posts - document all post fields with descriptions - include troubleshooting section - add best practices and tips
- add 17 useful commands for development workflow - include composite workflows (start, check, fix, prod) - provide shorter alternatives to pnpm commands - add cleanup and maintenance commands
- add content management section - document justfile commands - link to cms documentation - update command reference
- add detailed conventional commit guidelines - define commit types and rules - add examples of good and bad commits - specify no claude signatures policy
- update env var naming convention in cursorrules - rename all sanity env vars with TORUS_BLOG_ prefix - update .env.example with new variable names - update sanity.config.ts to use prefixed variables
- remove path reference to .astro/types.d.ts (auto-imported) - keep only types references which are valid - remove eslint triple-slash-reference exception - resolve linting issue properly instead of disabling rule
- add build args to dockerfile for sanity env vars - configure github actions ci with environment secrets - create comprehensive deployment guide - document setup for docker, vercel, netlify, cloudflare, railway - add troubleshooting section for common deployment issues
- replace deprecated deskTool with structureTool in sanity config - add proper type for vite plugins in astro config - fix content component type in postdetails layout - remove unused ts-expect-error directive - ensure htmlcontent handles undefined with fallback
- remove vite pluginoption import (not available in build context) - replace astro.astrocomponent with function type signature - simplify types to avoid external dependencies
- change output from static to hybrid for sanity studio support - add ts-ignore for astro content component type - add ts-ignore for tailwindcss vite plugin type compatibility - add content check before rendering to prevent invalid component errors - build now completes successfully with all pages generated
- change output to server with node adapter - add prerender true to all static pages - enable experimental session flag for sanity studio - add fallback values for sanity config to prevent build errors - all pages now prerendered except sanity studio admin route - build completes successfully with no errors
- add prerender to og image generation route - configure vite manual chunks for sanity studio - increase chunk size warning limit to 1000kb - resolve all prerender warnings except sanity studio route
3fbe2ae to
dab56b8
Compare
- Replace @ts-ignore with @ts-expect-error for better type safety - Update @tailwindcss/vite from 4.0.14 to 4.1.17 for improved compatibility - Refactor PostDetails.astro to use official Astro destructuring pattern - Remove unnecessary @ts-expect-error comment in PostDetails.astro This eliminates all ESLint and TypeScript check errors while following TypeScript best practices.
dab56b8 to
8aeccbf
Compare
- Replace mutable let with const in getAllPosts function - Use promise chaining instead of try/catch with reassignment - Add immutability guidelines to .cursorrules - Prefer functional patterns over imperative mutations
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntegrates Sanity CMS as a second content source, adds Sanity schemas/config, implements a getAllPosts aggregator to merge Markdown and Sanity posts, updates pages to use getAllPosts and export prerender, and adds CI/Docker env wiring, docs, TypeScript env declarations, and utilities for Sanity access and rendering. Changes
Sequence Diagram(s)sequenceDiagram
actor Browser
participant Page as Astro Page
participant Util as getAllPosts()
participant Markdown as Local Markdown
participant Sanity as Sanity CMS
participant Renderer as PostDetails.astro
Note over Page,Renderer: Page requests unified posts
Page->>Util: getAllPosts()
par fetch both sources
Util->>Markdown: getCollection("blog")
Util->>Sanity: getSanityPosts()
and
Markdown-->>Util: Markdown CollectionEntry[]
Sanity-->>Util: SanityBlogPost[]
end
Note over Util: Transform Sanity -> CollectionEntry (HTML conversion, ogImage handling)
Util-->>Page: Unified CollectionEntry[]
Page->>Renderer: render post
alt post.isSanity
Renderer->>Renderer: render <div set:html={htmlContent}>
else
Renderer->>Renderer: render <Content /> component
end
Renderer-->>Browser: HTML response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
- Fix archives page to use getAllPosts() instead of getCollection() - Ensures Sanity CMS posts appear in archives - Add runtime validation for Sanity environment variables - Validates required env vars when Sanity client is initialized - Avoids config-time validation issues with Astro - Add comprehensive error handling to Sanity functions - getSanityPosts() now has try-catch with descriptive errors - getSanityPostBySlug() has proper error context - Remove unused sanitize parameter from getAllPosts - Simplifies Content component render function - Improve type safety for ogImage field conversion - Add convertOgImage() helper with proper type guards - Handles ImageAsset to string/object conversion safely - Fix justfile lint error handling - Add || true to lint --fix command - Ensures format always runs even if lint fails - Revert output: 'hybrid' to 'server' - Hybrid mode not available in Astro 5.5.2 All changes maintain backward compatibility and improve code quality.
51f77fd to
e5c73ff
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 3
♻️ Duplicate comments (3)
src/pages/archives/index.astro (1)
2-19: LGTM: Archives page correctly migrated.The archives page now correctly uses
getAllPosts()instead ofgetCollection("blog"), ensuring Sanity CMS posts appear in the archives alongside Markdown posts. This resolves the inconsistency flagged in the previous review.src/lib/sanity.config.ts (1)
13-19: Configuration fallbacks violate project rules.The empty string fallbacks for
projectId(line 14) andtoken(line 18) violate the project's cursor rules that state: "Never use hardcoded fallback values in configuration files." This could lead to silent failures during initialization.While the comment mentions runtime validation, allowing the app to partially initialize with invalid configuration can cause confusing error messages later in the execution flow.
The previous review suggested throwing errors during configuration load:
const projectId = import.meta.env.PUBLIC_TORUS_BLOG_SANITY_PROJECT_ID; if (!projectId) { throw new Error( "Missing required environment variable: PUBLIC_TORUS_BLOG_SANITY_PROJECT_ID" ); } const token = import.meta.env.TORUS_BLOG_SANITY_API_TOKEN; if (!token) { throw new Error( "Missing required environment variable: TORUS_BLOG_SANITY_API_TOKEN" ); } export const SANITY_CONFIG = { projectId, dataset: import.meta.env.PUBLIC_TORUS_BLOG_SANITY_DATASET || "production", apiVersion: "2024-01-01", useCdn: false, token, studioBasePath: "/admin", } as const;Note: The comment on lines 10-11 mentions that env vars may not be available during Astro config load. If that's a concern, consider adding a validation function that runs during the first Sanity client initialization and provides clear error messages.
justfile (1)
43-46: Past review concern already addressed.The
fixrecipe already includes|| trueon line 46 to handle lint failures gracefully while ensuring format is always applied. The inline comment clearly documents this behavior.
🧹 Nitpick comments (9)
.env.example (1)
1-5: Align env key order with dotenv-linter expectationdotenv-linter suggests ordering the keys alphabetically; you can silence the warning by swapping the first two lines of config:
- PUBLIC_TORUS_BLOG_SANITY_PROJECT_ID=your-project-id - PUBLIC_TORUS_BLOG_SANITY_DATASET=production + PUBLIC_TORUS_BLOG_SANITY_DATASET=production + PUBLIC_TORUS_BLOG_SANITY_PROJECT_ID=your-project-id TORUS_BLOG_SANITY_API_TOKEN=your-api-tokenAlso good that only placeholders are committed here—no secrets.
src/pages/index.astro (1)
2-3: Check draft-handling consistency when usinggetAllPostsSwitching the home page to
getAllPosts()keeps the existing sorting/featured logic intact, which looks good.One potential gotcha: in
src/pages/posts/[...page].astroyou explicitly filter out drafts with!data.draftbefore paginating, but here you passpostsstraight intogetSortedPosts. IfgetAllPosts()includes drafts, they may show up on the homepage (and not in the paginated list), which would be confusing and could leak unpublished content.Consider either:
-const posts = await getAllPosts(); -const sortedPosts = getSortedPosts(posts); +const posts = (await getAllPosts()).filter(({ data }) => !data.draft); +const sortedPosts = getSortedPosts(posts);or moving the “published only” filtering into
getAllPosts/getSortedPostsso all pages share the same behavior.Also applies to: 11-11, 15-19
src/pages/tags/index.astro (1)
2-3: Ensure tags list is based on published posts onlyUsing
getAllPosts()andgetUniqueTags(posts)keeps this aligned with the rest of the data flow.Similar to the index page, though: if
getAllPosts()returns drafts, tags from draft posts may appear here even when the posts themselves are hidden in other views. That can expose in-progress taxonomy.Either:
- Filter
postsbefore computing tags:-const posts = await getAllPosts(); +const posts = (await getAllPosts()).filter(({ data }) => !data.draft);or
- Centralize “published-only” filtering inside
getAllPosts/getUniqueTagsso all tag consumers share the same behavior.Also applies to: 10-15
README.md (1)
3-7: Docs updates look good; clarify markdown vs CMS storageThe added sections for CMS/Deployment docs, content management modes, and Just commands are clear and helpful.
Minor wording nit: Line 43 currently says “All blog posts are stored in
src/data/blogdirectory.” With Sanity now supported, that’s no longer strictly true. Consider rephrasing to something like:-All blog posts are stored in `src/data/blog` directory. +Markdown blog posts are stored in the `src/data/blog` directory. +Sanity-managed posts live in your configured Sanity project.Also applies to: 45-59, 62-77, 81-90
docs/DEPLOYMENT.md (1)
51-94: Consider using runtime env vars for the Sanity token instead of build args in productionThe Docker instructions bake
TORUS_BLOG_SANITY_API_TOKENinto the image via--build-arg, which works but makes the token part of the built image. For production, it’s usually preferable to pass secrets as runtime env vars (e.g.,docker run -e TORUS_BLOG_SANITY_API_TOKEN=...orenvironment:indocker-compose.yml) so you can rotate them without rebuilding and keep images generic.The guide is otherwise clear and very helpful; you might just add a short note recommending runtime envs for secrets in production setups.
src/utils/getAllPosts.ts (2)
9-29: TightenconvertOgImagetypings and return typeThe implementation effectively only ever returns a string or
undefined, but the signature allows a{ src: string }and checks fortypeof ogImage === "string"even thoughogImageis typed asImageAsset | undefined.Consider aligning the types with the actual behavior, e.g.:
-function convertOgImage( - ogImage: ImageAsset | undefined -): string | { src: string } | undefined { +function convertOgImage( + ogImage: ImageAsset | string | undefined +): string | undefined { if (!ogImage) return undefined; if (typeof ogImage === "string") { return ogImage; } - if (typeof ogImage === "object" && "url" in ogImage) { - return ogImage.url as string; - } + if (typeof ogImage === "object" && "url" in ogImage && typeof ogImage.url === "string") { + return ogImage.url; + } return undefined; }This removes the unused
{ src }variant and makes the string branch type-correct if you ever decide to support string URLs from Sanity.
80-90: Swallowing Sanity fetch errors entirely may hide production issues
getAllPostsgracefully falls back to markdown-only content ifgetSanityPosts()fails:const sanityPosts = await getSanityPosts() .then(sanityData => sanityData.map(sanityPostToCollectionEntry)) .catch(() => [] as CollectionEntry<"blog">[]);This is good for resilience, but because the error is ignored, diagnosing misconfigured credentials or network issues could be harder.
Consider at least logging the error (or sending it to your observability stack) before returning
[], for example:- const sanityPosts = await getSanityPosts() - .then(sanityData => sanityData.map(sanityPostToCollectionEntry)) - .catch(() => [] as CollectionEntry<"blog">[]); + const sanityPosts = await getSanityPosts() + .then(sanityData => sanityData.map(sanityPostToCollectionEntry)) + .catch(error => { + console.error("Failed to fetch Sanity posts, serving markdown-only content:", error); + return [] as CollectionEntry<"blog">[]; + });That keeps the “fail open” behavior while still surfacing issues.
src/lib/sanity.ts (1)
26-57: Good runtime config validation and clearer Sanity fetch errors – be aware of eager failure behavior
validateSanityConfig()plus the descriptive try/catch blocks ingetSanityPostsandgetSanityPostBySlugare a solid improvement: missing project/dataset/token will fail fast with a clear message, and fetch errors now carry useful context (Failed to fetch Sanity blog posts…/…with slug "…") instead of bubbling up opaque client errors.One thing to keep in mind: because
validateSanityConfig()runs at module import time, any environment (including local tools/tests that import this module) must have all three env vars set or it will throw immediately. If you ever want the app to run in a “Markdown-only” mode without Sanity configured, you might move the validation intogetSanityPosts/getSanityPostBySlugand let callers decide how to handle failures.Functionally, though, this is a good, explicit failure mode for a Sanity-dependent deployment.
Also applies to: 59-86, 88-117
justfile (1)
59-66: Excellent workflow recipes for different scenarios.The composite workflows (
start,pre-commit,prod) provide well-thought-out shortcuts for common development tasks:
start: Complete setup for new developerspre-commit: Fast checks for commits (omits build for speed)prod: Full production build workflowOptional: Consider documenting why
pre-commitexcludes the build step.The
pre-commitrecipe intentionally omits thebuildstep to keep checks fast. You might consider adding a comment explaining this trade-off, or adding an optionalpre-pushrecipe that includesbuildfor more comprehensive pre-deployment checks:# Pre-push checks: lint, format, type check, and build pre-push: lint format-check sync build
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.cursorrules(1 hunks).env.example(1 hunks).github/workflows/ci.yml(1 hunks)Dockerfile(1 hunks)README.md(2 hunks)astro.config.ts(3 hunks)docs/CMS.md(1 hunks)docs/DEPLOYMENT.md(1 hunks)justfile(1 hunks)package.json(1 hunks)sanity.config.ts(1 hunks)schemaTypes/blogPost.ts(1 hunks)schemaTypes/index.ts(1 hunks)src/data/blog/the-ship-of-theseus-and-the-persistence-of-living-form.md(2 hunks)src/data/blog/understanding-torus.md(2 hunks)src/env.d.ts(1 hunks)src/layouts/PostDetails.astro(2 hunks)src/lib/sanity.config.ts(1 hunks)src/lib/sanity.ts(1 hunks)src/pages/404.astro(1 hunks)src/pages/archives/index.astro(1 hunks)src/pages/index.astro(1 hunks)src/pages/posts/[...page].astro(1 hunks)src/pages/posts/[...slug]/index.astro(2 hunks)src/pages/posts/[...slug]/index.png.ts(2 hunks)src/pages/robots.txt.ts(1 hunks)src/pages/rss.xml.ts(1 hunks)src/pages/search.astro(1 hunks)src/pages/tags/[tag]/[...page].astro(2 hunks)src/pages/tags/index.astro(1 hunks)src/utils/getAllPosts.ts(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 4-4: [UnorderedKey] The PUBLIC_TORUS_BLOG_SANITY_DATASET key should go before the PUBLIC_TORUS_BLOG_SANITY_PROJECT_ID key
(UnorderedKey)
🔇 Additional comments (33)
src/data/blog/the-ship-of-theseus-and-the-persistence-of-living-form.md (3)
12-12: Front matter and formatting improvements look good.The whitespace cleanup on the tag (line 12), blank line after the front matter delimiter (line 14), and spacing before the image block (line 16) all improve consistency and readability. These changes align with standard Markdown conventions.
Also applies to: 14-16
30-30: Correct list marker removal and emphasis consistency.Line 30 properly removes the errant leading dash, converting it from a stray list item into a proper paragraph. The change from asterisk to underscore emphasis for "remember" is stylistically consistent and maintains semantic correctness—both markers are valid Markdown emphasis.
1-14: No issues found—front matter and image path are correctly configured.Verification confirms:
- Image file
the-ship-of-theseus.jpgexists atsrc/assets/images/- Path alias
@/*→./src/*is properly defined intsconfig.json, so@/assets/images/the-ship-of-theseus.jpgresolves correctly- All front matter fields (title, author, pubDatetime, featured, draft, ogImage, tags, description) are present and aligned with the schema used in other blog posts
src/env.d.ts (1)
1-2: LGTM! Proper TypeScript configuration.The ambient reference directives correctly enable type definitions for both Astro client APIs and the Sanity Astro module integration.
src/data/blog/understanding-torus.md (2)
6-13: LGTM! Improved tags formatting.The multi-line YAML array format improves readability and makes individual tag changes easier to track in version control.
22-22: LGTM! Valid emphasis syntax.The italic emphasis appropriately highlights the introductory paragraph.
schemaTypes/index.ts (1)
1-3: LGTM! Clean schema aggregator.This follows the standard Sanity pattern for exporting schema types. The structure allows easy addition of new schema types in the future.
docs/CMS.md (2)
193-208: LGTM! Field reference matches schema.The fields reference table accurately reflects the schema defined in
schemaTypes/blogPost.ts, including types, required status, and validation rules.
1-378: Excellent comprehensive documentation.This CMS guide is well-structured, thorough, and user-friendly. It covers all necessary topics with clear examples, troubleshooting guidance, and helpful references. This will significantly improve the content creator experience.
.cursorrules (3)
18-25: LGTM! Strong immutability guidelines.The emphasis on immutability and functional patterns will reduce bugs and improve code maintainability. This is excellent guidance for TypeScript development.
149-164: LGTM! Strong environment variable conventions.The strict naming conventions and prohibition of hardcoded fallbacks are excellent security practices. The
TORUS_BLOG_prefix helps avoid conflicts with other projects.
83-85: LGTM! Important git hygiene rule.Explicitly forbidding AI signatures in commit messages ensures professional, clean git history. This is a valuable guideline for AI-assisted development workflows.
schemaTypes/blogPost.ts (2)
8-89: LGTM! Well-defined schema with appropriate validation.The field definitions and validation rules are well-structured:
- Required fields correctly identified (title, slug, pubDatetime, description, content)
- Slug maxLength of 96 prevents URL issues
- Description max 200 chars is optimal for SEO meta descriptions
- Default values are sensible (author: "Torus", booleans: false)
- Field descriptions provide clear guidance
114-138: LGTM! Clean preview implementation.The preview configuration effectively shows post status:
- Uses
filter(Boolean)to elegantly handle optional labels- Combines author, draft status, and featured indicator with bullet separators
- Provides clear visual feedback in the Sanity Studio list view
src/pages/robots.txt.ts (1)
1-1: LGTM! Appropriate static prerendering.Adding
prerender = truecorrectly enables static generation for the robots.txt file, which is optimal since this file doesn't require dynamic server-side rendering. This aligns with Astro v5's prerendering patterns..github/workflows/ci.yml (1)
47-50: Verify GitHub Secrets are configured (manual verification required).The automated check could not verify these secrets due to GitHub API permissions. Please manually confirm that the following three secrets are configured in your GitHub repository settings at
https://github.com/renlabs-dev/torus-blog/settings/secrets/actions:
PUBLIC_TORUS_BLOG_SANITY_PROJECT_IDPUBLIC_TORUS_BLOG_SANITY_DATASETTORUS_BLOG_SANITY_API_TOKENThe naming follows project conventions, and the
PUBLIC_prefix correctly designates client-accessible values whileTORUS_BLOG_SANITY_API_TOKENremains server-only.src/pages/search.astro (1)
2-2: Prerendering search route looks appropriateMaking the search page explicitly prerendered is consistent with Pagefind’s static index flow and the other pages in this PR; no issues spotted.
src/pages/404.astro (1)
2-2: Static 404 prerender is a good explicit choiceExplicitly prerendering the 404 page matches the rest of the site’s static strategy and avoids surprises in static hosting setups.
src/pages/posts/[...page].astro (1)
2-3: Unified posts data source with explicit draft filtering looks solidUsing
getAllPosts()as the single source of posts and explicitly filtering out drafts before sorting and paginating is a clean approach and aligns with the intent of keeping unpublished content hidden. The explicitprerender = truealso makes the static behavior clear.If you’d like to further DRY things up, you could consider a small helper (e.g.,
getPublishedPosts) that wrapsgetAllPosts()+!data.draftto reuse across index/archives/tags without duplicating the filter.Also applies to: 12-19
Dockerfile (1)
13-21: Sanity build args and token handling verified as secureVerification confirms the token is properly confined to server-side contexts only:
getSanityPosts()is called only withingetAllPosts()utilitygetAllPosts()is invoked exclusively in Astro server endpoints (rss.xml.ts,index.png.ts)- Token is accessed via
import.meta.env.TORUS_BLOG_SANITY_API_TOKEN(server-side variable withoutPUBLIC_prefix)- No client-side imports from
sanity.tsorsanity.config.tsastro.config.tsusesoutput: "server"with Node adapter, confirming full SSR architectureThe token never reaches the browser bundle.
sanity.config.ts (2)
4-4: Verify import path convention.The root-level
sanity.config.tsis importing from./src/lib/sanity.config, which is unconventional. Typically, root configuration files import from packages or relative sibling directories, not from thesrc/directory. While this works, it creates a dependency from root config to application code.Consider whether this import path aligns with your project's module organization conventions. If this is intentional for centralized config management, ensure the pattern is documented.
6-14: LGTM: Clean Sanity Studio configuration.The Sanity Studio configuration correctly uses the centralized
SANITY_CONFIGfor project details and includes the necessary structure plugin and schema types.src/pages/posts/[...slug]/index.png.ts (1)
1-23: LGTM: Correct migration to unified data source.The changes properly migrate from
getCollection("blog")togetAllPosts()and add static prerendering. The filtering and mapping logic for OG image generation remains intact.src/pages/rss.xml.ts (1)
1-23: LGTM: RSS feed correctly updated.The RSS feed generation properly switches to
getAllPosts()and enables static prerendering, ensuring Sanity CMS posts will be included in the feed alongside Markdown posts.src/pages/tags/[tag]/[...page].astro (1)
2-28: LGTM: Tag pagination correctly migrated.The tag-based pagination pages properly adopt
getAllPosts()for unified content sourcing and enable static prerendering. The tag filtering and pagination logic remains intact.src/pages/posts/[...slug]/index.astro (1)
2-28: LGTM: Post detail pages correctly migrated.The post detail page properly adopts
getAllPosts()in bothgetStaticPathsand the main rendering logic, ensuring unified content sourcing and static prerendering.package.json (1)
16-40: Dependency upgrades are compatible—no issues found.Verification confirms all major version upgrades are safe:
- @astrojs/react v4.4.2 supports React 19
- No React components using deprecated patterns (0
.tsx/.jsxfiles in codebase)- Tailwind config properly uses
@tailwindcss/vitefor v4 compatibility- No deprecated Tailwind syntax found in
.astroor.cssfiles- Astro v5 integration configured correctly
The PR is ready as-is.
src/layouts/PostDetails.astro (2)
36-47: Sanity-vs-markdown branching and render usage look soundGating
render(post)behind!isSanityPostavoids calling Astro’s content renderer on the synthetic Sanity entries fromgetAllPosts, which is the right separation. Thepost.bodypath for Sanity andrender(post)path for markdown posts are clearly split and align with how those entries are constructed.
133-141: Conditional tags block avoids empty markupGuarding the tag list with
tags && tags.length > 0is a nice cleanup and prevents rendering an empty<ul>when there are no tags.astro.config.ts (1)
6-8: Astro server mode, Node adapter, and Sanity/React wiring look consistent with Astro 5Using
output: "server"with the Node adapter in standalone mode is the right pattern for enabling SSR and the/adminstudio while still letting prerendered pages remain static. The Sanity integration parameters pulled fromSANITY_CONFIGand the separatesanity-studiomanual chunk should help keep the main bundle lean when the Studio is loaded.Enabling
experimental.session: trueis fine as long as you actually rely on sessions; otherwise it’s just an extra feature surface. Nothing blocking here, but it’s worth double‑checking that this matches your deployment/runtime expectations.Also applies to: 10-10, 15-18, 20-27, 40-41, 45-55, 66-66
justfile (3)
1-6: LGTM! Clear documentation and helpful default behavior.The file header and default recipe follow best practices. The reference to the
justdocumentation and the auto-generated help display make the file discoverable and easy to use.
8-38: LGTM! Well-structured basic recipes with clear documentation.All basic recipes follow a consistent pattern, are properly documented, and map cleanly to pnpm scripts. The inclusion of the
syncrecipe for TypeScript type generation is especially important for Astro projects.
48-54: Clean recipes are well-structured.The separation between
clean(build artifacts) andclean-all(including node_modules) provides useful granularity. The use ofrm -rfis standard, though users on Windows may need WSL, Git Bash, or a similar Unix-like environment.
- Make Sanity client creation lazy (only when actually used) - Return empty arrays/null when Sanity is not configured - Use placeholder fallbacks in config for Astro/Sanity integration build - Add clear documentation explaining why placeholders are needed This fixes the GitHub Actions build error while maintaining proper validation when Sanity CMS is actually configured. The placeholder values allow the Astro config and Sanity integration to load during build time when env vars may not be available (e.g., CI environment). Runtime validation in sanity.ts prevents actual usage without proper configuration.
- Add image support to blog post content with alt text and captions - Add TypeScript typing for Sanity environment variables - Hide "Suggest Changes" button for Sanity posts (managed in CMS) - Remove all hardcoded environment variable fallbacks - Configure Sanity to use direct env vars for better security - All configuration now comes exclusively from .env file Environment variables are now properly typed: - PUBLIC_TORUS_BLOG_SANITY_PROJECT_ID: string (required) - PUBLIC_TORUS_BLOG_SANITY_DATASET: "production" | "dev" | "stage" - TORUS_BLOG_SANITY_API_TOKEN: string (required)
- Add draft filtering on home and tags pages to prevent unpublished content - Add error logging for Sanity fetch failures for production debugging - Change let to const in tags page for immutability - Verify pnpm cache already configured in GitHub Actions
b034a45 to
fc008ca
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
Copilot reviewed 30 out of 32 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 3
♻️ Duplicate comments (2)
justfile (1)
43-46: Past review concern has been addressed.The past review comment raised a concern about the
fixrecipe stopping if lint fails. This has been properly addressed by:
- Adding
|| trueto line 46, ensuring the recipe completes even if lint fails- Documenting the behavior in the comment at line 44
src/utils/getAllPosts.ts (1)
40-100: Customrender()andas CollectionEntry<"blog">are unsafe and fragileTwo related concerns here:
- The synthetic
renderimplementation returns a React‑style element shape ({ type: "div", props: { … } }) rather than an AstroContentcomponent, so it doesn’t actually fulfill theCollectionEntry<"blog">["render"]contract.- The
as CollectionEntry<"blog">cast then masks the mismatch, so future callers may legitimately callpost.render()on a Sanity post and hit confusing runtime behavior.Given current usage where Sanity posts are rendered via
bodyrather thanpost.render(), a safer pattern would be:
- Make
render()explicitly a no‑op / unsupported implementation (e.g.,Content() { return null; }, empty headings/frontmatter) and document thatrender(post)is only valid for markdown entries; or- Introduce a narrower shared type (e.g.,
UnifiedPost) that doesn’t promise a workingrendermethod for Sanity‑backed posts.That way the type system doesn’t claim capabilities that aren’t really there, and future refactors won’t accidentally rely on a broken
render().
🧹 Nitpick comments (6)
justfile (1)
59-60: Consider adding error handling for the start workflow.The
startrecipe sequencesinstall sync dev, but ifinstallorsyncfails, the error may not be immediately obvious sincedevwon't start. While the current behavior is correct (stopping on errors), consider adding explicit error messages or checks.Optionally, you can add error handling to make failures more visible:
# Development workflow: install, sync, and start dev server -start: install sync dev +start: + just install + just sync + just devThis separates the commands so that Just will stop and report which step failed more clearly.
src/utils/getAllPosts.ts (2)
17-37: TightenconvertOgImagetyping and behaviorThe function signature and implementation don’t quite line up:
- Parameter is typed as
ImageAsset | undefined, but you branch ontypeof ogImage === "string", which can never be true with that type.- The return type includes
{ src: string }, but no branch actually returns that shape; all current branches return a string orundefined.Might be cleaner (and safer) to either:
- Widen the parameter type to what
SanityBlogPost["ogImage"]actually is (e.g.,string | ImageAsset | { src: string } | undefined) and add a{ src }branch if needed, or- Narrow the implementation to only handle
ImageAsset | undefinedand drop the unused string /{ src }cases.This will make the intent clearer and avoid dead branches or misleading types.
105-122: Optional: fetch markdown and Sanity posts in parallelFunctionally this works fine, including the error fallback, but you currently:
await getCollection("blog")- then
await getSanityPosts().then(...).catch(...)If build‑time latency ever matters, you could start both in parallel:
-export async function getAllPosts(): Promise<CollectionEntry<"blog">[]> { - const markdownPosts = await getCollection("blog"); - const sanityPosts = await getSanityPosts() - .then((sanityData) => sanityData.map(sanityPostToCollectionEntry)) - .catch((error) => { - console.error("Failed to fetch Sanity posts, serving markdown-only content:", error); - return [] as CollectionEntry<"blog">[]; - }); - return [...markdownPosts, ...sanityPosts]; -} +export async function getAllPosts(): Promise<CollectionEntry<"blog">[]> { + const [markdownPosts, sanityPosts] = await Promise.all([ + getCollection("blog"), + getSanityPosts() + .then((sanityData) => sanityData.map(sanityPostToCollectionEntry)) + .catch((error) => { + console.error("Failed to fetch Sanity posts, serving markdown-only content:", error); + return [] as CollectionEntry<"blog">[]; + }), + ]); + return [...markdownPosts, ...sanityPosts]; +}Not critical, but an easy win if Sanity latency is noticeable.
schemaTypes/blogPost.ts (1)
1-161: Schema is well-structured and matches the unified post model (with one minor mismatch)The
blogPostschema fields/validation line up cleanly with howSanityBlogPostis consumed insanityPostToCollectionEntry(title, slug, dates, flags, tags, description, content, ogImage, canonicalURL), and the preview config is a nice touch for editors.One small mismatch: the schema exposes
hideEditPost, but the conversion code currently hardcodeshideEditPost: truefor all Sanity posts. If you don’t plan to let editors control that flag from Studio, consider dropping it from the schema; if you do, wire it through in the mapper so Studio behavior matches the content model.src/lib/sanity.ts (2)
63-95: Consider extracting the GROQ projection to reduce duplication.The field selection in this query is duplicated in
getSanityPostBySlug(lines 105-122). Extracting it to a constant improves maintainability and ensures consistency.Apply this diff to extract the projection:
+const BLOG_POST_PROJECTION = `{ + _id, + _type, + title, + slug, + author, + pubDatetime, + modDatetime, + featured, + draft, + unlisted, + tags, + description, + content, + ogImage, + canonicalURL, + hideEditPost +}`; + export async function getSanityPosts(): Promise<SanityBlogPost[]> { // Return empty array if Sanity is not configured if (!isSanityConfigured()) { return []; } - const query = groq`*[_type == "blogPost"] | order(pubDatetime desc) { - _id, - _type, - title, - slug, - author, - pubDatetime, - modDatetime, - featured, - draft, - unlisted, - tags, - description, - content, - ogImage, - canonicalURL, - hideEditPost - }`; + const query = groq`*[_type == "blogPost"] | order(pubDatetime desc) ${BLOG_POST_PROJECTION}`;Then update
getSanityPostBySlugsimilarly:- const query = groq`*[_type == "blogPost" && slug.current == $slug][0] { - _id, - _type, - title, - slug, - author, - pubDatetime, - modDatetime, - featured, - draft, - unlisted, - tags, - description, - content, - ogImage, - canonicalURL, - hideEditPost - }`; + const query = groq`*[_type == "blogPost" && slug.current == $slug][0] ${BLOG_POST_PROJECTION}`;
88-94: Consider adding runtime type validation.The fetched data is assumed to match the
SanityBlogPostinterface without runtime validation. While this is common with Sanity, adding validation (e.g., using Zod or a similar library) would catch schema mismatches and provide better error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
astro.config.ts(2 hunks)justfile(1 hunks)package.json(1 hunks)sanity.config.ts(1 hunks)schemaTypes/blogPost.ts(1 hunks)src/env.d.ts(1 hunks)src/lib/sanity.config.ts(1 hunks)src/lib/sanity.ts(1 hunks)src/pages/index.astro(1 hunks)src/pages/tags/index.astro(1 hunks)src/utils/getAllPosts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- sanity.config.ts
- src/lib/sanity.config.ts
- src/pages/index.astro
- src/pages/tags/index.astro
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (8)
justfile (1)
65-66: Production workflow looks solid.The
prodrecipe correctly sequencesclean install sync buildto ensure a fresh, reproducible production build. This is the right approach for CI/CD pipelines.src/env.d.ts (1)
1-12: Env typings for Sanity integration look consistentThe
ImportMetaEnv/ImportMetadeclarations line up with the Sanity config usage and give you nice type‑safety around the required env vars; no issues spotted here.package.json (1)
16-43: New dependencies match the Sanity + React integration surfaceThe added runtime deps (
@sanity/*,sanity,groq, React +@astrojs/react,styled-components, etc.) all line up with wiring in the rest of the PR (Sanity Studio, Sanity client, image URLs, React integration). Nothing stands out as obviously misplaced here.If you want extra confidence, you could run a quick scan to ensure each new dep is actually imported somewhere (and catch any stragglers left over from experimentation).
astro.config.ts (2)
6-25: Sanity + React integration andoutput: "static"look coherentUsing
output: "static"together withsanity()(dev‑only Studio per comment) andreact()is consistent with Astro v5’s model: the site remains statically generated while Sanity content is fetched at build time and Studio is accessed via the dev server.The env‑backed
projectId/datasetwith sensible fallbacks should also keep local builds from breaking when vars are missing.
38-53: Manual chunk for Sanity Studio is a sensible bundle-splitting strategyCustomizing
vite.build.rollupOptions.output.manualChunksto put"sanity-studio": ["sanity"]plus raisingchunkSizeWarningLimitis a pragmatic way to keep Studio code out of the main app bundle and tame build warnings.If you see Studio pulling in additional large deps later, you can extend this chunk list, but the current setup is a solid starting point.
src/lib/sanity.ts (3)
1-24: LGTM!The imports and interface definition are well-structured. The
SanityBlogPostinterface properly uses Sanity's official types and clearly distinguishes required from optional fields.
97-131: LGTM!The error handling is properly implemented with descriptive messages including the slug for debugging. The graceful degradation pattern (returning
nullwhen not configured) is appropriate for this optional integration.
42-61: The code is correctly configured. When using an authenticated token,useCdnshould be set tofalse, and the configuration already implements this. Additionally, theapiVersioncorrectly uses Sanity's date-based format (YYYY-MM-DD). The implementation follows Sanity's best practices for authenticated access.
- astro: 5.5.2 → 5.16.0 - @astrojs/node: 9.5.0 → 9.5.1 - @astrojs/rss: 4.0.11 → 4.0.14 - @astrojs/sitemap: 3.2.1 → 3.6.0 - @astrojs/check: 0.9.4 → 0.9.5
- Remove graduated experimental flags (svg, responsiveImages, preserveScriptOrder) - Update image layout from experimentalLayout to stable layout config - Change image layout value from "responsive" to "constrained" - Fix SVG component props in Header (remove custom size/title props) - Use standard HTML attributes and Tailwind classes for SVG styling
- Add HTML escaping for image alt/caption to prevent XSS attacks - Remove hardcoded fallback values from runtime sanity config - Strengthen isSanityConfigured validation with trim and placeholder checks - Add security comment explaining --ignore-scripts usage in justfile - Keep build-time placeholders in astro.config.ts for compatibility
|
Re: The Why this approach:
Added explanatory comments in the justfile to document this security decision. |
|
Re: Hardcoded fallback values in The fallback values in Why we keep them in astro.config.ts:
How we handle runtime:
This approach ensures:
|
- Add sanitize-html to prevent XSS from compromised Sanity accounts - Configure allowed tags and attributes for blog content safety - Simplify redundant Content check in PostDetails layout - Remove unverified Publish keyboard shortcut from CMS docs - Sanitize HTML after portable text conversion for defense-in-depth
- Add studioBasePath: "/admin" to sanity integration config - Fixes /admin endpoint that was broken after config changes - Studio now properly served in dev mode at /admin route
Changed the Sanity Studio path to /studio to avoid common bot targets like /admin. This provides basic security through obscurity while keeping the path memorable for authorized users. Changes: - Updated studioBasePath in astro.config.ts and src/lib/sanity.config.ts - Updated all documentation references in docs/CMS.md - Switched from static to server output mode with Node.js adapter (required for Sanity Studio SSR routes in Astro 5) - All blog pages remain prerendered (static) for optimal performance
85f738c to
5848a74
Compare
EdSDR
left a 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.

Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.